Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: state behavior for url pathname and percent encoding #1538

Closed
gopat opened this issue Apr 27, 2015 · 1 comment
Closed

docs: state behavior for url pathname and percent encoding #1538

gopat opened this issue Apr 27, 2015 · 1 comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. url Issues and PRs related to the legacy built-in url module.

Comments

@gopat
Copy link

gopat commented Apr 27, 2015

Shouldn't be stated in the docs that url module does NOT touch pathname or pathname part of path (no percent encoding/decoding) when extracting-from/putting-into urls?

> url.parse('https://example.com/pathname%20with%20fun%20incorporated%3d/?adf=f4&er4=%40',false);
{ protocol: 'https:',
  slashes: true,
  auth: null,
  host: 'example.com',
  port: null,
  hostname: 'example.com',
  hash: null,
  search: '?adf=f4&er4=%40',
  query: 'adf=f4&er4=%40',
  pathname: '/pathname%20with%20fun%20incorporated%3d/',
  path: '/pathname%20with%20fun%20incorporated%3d/?adf=f4&er4=%40',
  href: 'https://example.com/pathname%20with%20fun%20incorporated%3d/?adf=f4&er4=%40' }

> url.parse('https://example.com/pathname%20with%20fun%20incorporated%3d/?adf=f4&er4=%40',true);
{ protocol: 'https:',
  slashes: true,
  auth: null,
  host: 'example.com',
  port: null,
  hostname: 'example.com',
  hash: null,
  search: '?adf=f4&er4=%40',
  query: { adf: 'f4', er4: '@' },
  pathname: '/pathname%20with%20fun%20incorporated%3d/',
  path: '/pathname%20with%20fun%20incorporated?adf=f4&er4=%40',
  href: 'https://example.com/pathname%20with%20fun%20incorporated?adf=f4&er4=%40' }

In fact, it does not do any percent encoding/decoding at all in query/search -with the exception of opt-in querystring parsing (parseQueryString argument) or when encoding an object including query params to a full url-.
In this case (querystring/search whatever you call it) it is pretty obvious (decoding without parsing leads to incorrect data, ie: '?a=%26b'), and it is kind of implicitly documented in the examples, though.

But stating it clearly in the docs for pathname and pathname part in path should let api users avoid the 'implemetation' vs 'intended' behavior dilemma: trusty, future-proof('till api change) behavior.

I'm ssuming that it is intended behavior. If it happens to be an implementation detail... i'm not related to the project but if asked would be +1 on current behavior: raw original data available, that can be decoded if needed, so we don't lose upper/lowercase info for percent escapes (of significance to oauth1.0a).

Notes: http module: url property in IncomingMessages (http.Server 'request' event) is consistent with url module behavior right now. "Fix" docs too ?

PD: My humble apologies for the poor quality of my mind's English implementation.

@mscdex mscdex added doc Issues and PRs related to the documentations. url Issues and PRs related to the legacy built-in url module. labels Apr 27, 2015
@Fishrock123 Fishrock123 added the good first issue Issues that are suitable for first-time contributors. label Apr 28, 2015
@Fishrock123 Fishrock123 changed the title API Documentation: url module, state behavior with respect to pathname and percent encoding docs: state behavior for url pathname and percent encoding Apr 28, 2015
silverwind pushed a commit that referenced this issue May 25, 2015
Explicitly states the fact that no decoding is performed on the url
path or pathname or the query string by default in the URL module.

Fixes: #1538
PR-URL: #1731
Reviewed-By: Roman Reiss <me@silverwind.io>
@silverwind
Copy link
Contributor

Fixed by a74c2c9

andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this issue Jun 3, 2015
Explicitly states the fact that no decoding is performed on the url
path or pathname or the query string by default in the URL module.

Fixes: nodejs/node#1538
PR-URL: nodejs/node#1731
Reviewed-By: Roman Reiss <me@silverwind.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants