Skip to content

Incorrect expansion of multiple query variables #108

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

Closed
phodopus42 opened this issue Apr 24, 2023 · 3 comments
Closed

Incorrect expansion of multiple query variables #108

phodopus42 opened this issue Apr 24, 2023 · 3 comments

Comments

@phodopus42
Copy link

phodopus42 commented Apr 24, 2023

Version: 4.1.1. Platform: Fedora 37 x86_64 (though this is not platform dependant).

tl;dr: With multiple query variables, URITemplate appends a ? instead of a &. This is in violation of RFC6570 (IIUC).

Expected:

>>> URITemplate("http://example.org{?foo}{?bar}").expand(foo="this", bar="that")
'http://example.org?foo=this&bar=that'

(Second variable has a continuation.)

Actual:

>>> URITemplate("http://example.org{?foo}{?bar}").expand(foo="this", bar="that")
'http://example.org?foo=this?bar=that'

(Second variable has a question mark -- incorrect.)

See https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.8

For each defined variable in the variable-list:

o append "?" to the result string if this is the first defined value
or append "&" thereafter;

The form {&foo} is only for those templates that have a literal ?. See https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.9

Form-style query continuation, as indicated by the ampersand ("&")
operator in Level 3 and above templates, is useful for describing
optional &name=value pairs in a template that already contains a
literal query component with fixed parameters.

I can provide a patch later if required.

@Klupamos
Copy link

Klupamos commented Apr 26, 2023

I don't actually think this is a bug.
Your example template http://example.org{?foo}{?bar} has two seperate variable expressions, each with a variable-list containing only one element.
Section #2.2 & Section #2.3

What you are describing would be achieved with the template http://example.org{?foo,bar}

@sigmavirus24
Copy link
Collaborator

We could raise an exception when we parse and find two variables like this that are conflicting but that's a lot more work than we currently do and would require a backwards incompatible release

@phodopus42
Copy link
Author

My bad. Yes, I think you are correct.

I misinterpreted the statement in the spec "if this is the first defined value" to mean "across all expansions".

If you make it throw in this case, you will have to also consider all other cases where a valid template (according to 6570) can produce a invalid URI under some combination of variable expansions; I don't see that as practical.

Please feel free to close.

@sigmavirus24 sigmavirus24 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 28, 2023
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

3 participants