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

Add a function to resolve URI references relative to a base URI #19

Merged
merged 7 commits into from
Apr 30, 2021

Conversation

kernelmethod
Copy link
Contributor

RFC 3986 Section 5.2 (https://tools.ietf.org/html/rfc3986#section-5.2) defines an method for resolving references relative to a base URI. This pull request adds the resolvereference function in order to achieve this functionality, which attempts to comply with the algorithm defined in Section 5.2.2.

Closes #18.

Adds a new function, `resolvereference`, that resolves references
between a base URI and a reference URI. This function attempts to comply
with RFC 3986 Section 5.2 (https://tools.ietf.org/html/rfc3986#section-5.2).

Closes JuliaWeb#18.
- Expand the documentation for resolvereference and add some examples
  for it.
- Add resolvereference to the public documentation.
Change the version of `findprev` that `resolvereference` uses from
`findprev(::AbstractChar, ::AbstractString, ::Integer)` to
`findprev(::AbstractString, ::AbstractString, ::Integer)` (since the
former has only been available since Julia 1.3).
@fredrikekre
Copy link
Member

I believe that this is what URIs.absuri tries to do, but this seems more general. I propose to deprecated URIs.absuri in favor of this function.

This will also close JuliaWeb/HTTP.jl#435 and JuliaWeb/HTTP.jl#626 and maybe #21 ?

src/URIs.jl Outdated Show resolved Hide resolved
src/URIs.jl Outdated Show resolved Hide resolved
src/URIs.jl Outdated Show resolved Hide resolved
src/URIs.jl Outdated Show resolved Hide resolved
@fredrikekre fredrikekre requested review from quinnj and c42f April 29, 2021 22:27
Copy link
Member

@quinnj quinnj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kernelmethod
Copy link
Contributor Author

kernelmethod commented Apr 30, 2021

Thanks for the review @fredrikekre, I've applied the changes you've suggested

Edit: also, I've tried the example from #21 and I've confirmed that using resolvereference will get the expected result, so this should close that issue too.

@fredrikekre
Copy link
Member

I've tried the example from #21 and I've confirmed that using resolvereference will get the expected result, so this should close that issue too.

But only if you used resolvereference instead of joinpath, right? Perhaps joinpath should be updated to call resolvereference?

@fredrikekre fredrikekre merged commit d481cf3 into JuliaWeb:master Apr 30, 2021
@fredrikekre fredrikekre mentioned this pull request Apr 30, 2021
@kernelmethod
Copy link
Contributor Author

kernelmethod commented Apr 30, 2021

But only if you used resolvereference instead of joinpath, right?

Yes, correct.

Perhaps joinpath should be updated to call resolvereference?

Possibly? resolvereference and joinpath are quite similar in their purpose (as I noted in #18), but resolvereference explicitly complies with RFC 3986 Sec. 5.2 (and hence with URI resolution methods in other languages, including the JavaScript example from #21). It'd probably be nice if joinpath called resolvereference under the hood, but I'm not sure if there's a way to do so that doesn't break the current behavior of joinpath.

If we aren't worried about breaking joinpath's behavior, though, we could probably very easily redefine it as

Base.joinpath(base::URI, parts...) = reduce(resolvereference, [base, parts...])

(or something similar).

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

Successfully merging this pull request may close these issues.

Feature request: resolvereference function to resolve relative and absolute URIs
3 participants