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

Do URL and path joining with urljoin #58

Closed
mhrivnak opened this issue Dec 17, 2015 · 2 comments
Closed

Do URL and path joining with urljoin #58

mhrivnak opened this issue Dec 17, 2015 · 2 comments
Assignees

Comments

@mhrivnak
Copy link
Contributor

Many places in the tests combine URLs and paths with simple string addition. This has the potential to produce incorrect results.

Pure addition of path parts is dangerous, because standard "urljoin" logic, which follows rules in various RFCs, behaves differently. For example:

>>> '/a/b/c' + '/d/e'
'/a/b/c/d/e'
>>> urljoin('/a/b/c', '/d/e')
'/d/e'

Pulp should produce paths that behave correctly with urljoin, and we've had problems in the past with developers making "relative paths" that have a leading slash.

Relative paths with leading slashes are just one demonstration of how the behavior is different. Here is another where string addition would produce an incorrect result:

>>> urljoin('http://foo.com', 'a/b/c')
'http://foo.com/a/b/c'

Example of the undesirable use of string addition: https://github.com/PulpQE/pulp-smash/blob/668f4b7fd5e88c5f9290e4d3b251be587e026f43/pulp_smash/utils.py#L106

I expect most clients of our REST API will use a urljoin implementation to combine paths, and pulp-smash should do the same.

@Ichimonji10
Copy link
Contributor

Pulp should produce paths that behave correctly with urljoin, and we've had problems in the past with developers making "relative paths" that have a leading slash.

By using urljoin to handle the hrefs returned by Pulp, we'll be testing that the hrefs are correctly formed. Out of the points made above, this one makes the most sense to me. 👍

@Ichimonji10 Ichimonji10 self-assigned this Dec 18, 2015
@Ichimonji10
Copy link
Contributor

FYI, as of Python 3.5:

The parse.urljoin() was updated to use the RFC 3986 semantics for the resolution of relative URLs, rather than RFC 1808 and RFC 2396. (Contributed by Demian Brecht and Senthil Kumaran in issue 22118.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants