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 not expand undefined URI template variables. #161

Merged

Conversation

gabesullice
Copy link
Contributor

@gabesullice gabesullice commented Feb 29, 2020

Hello 👋! I am really excited to have found that the league has an implementation of [RFC 6570 - URI Template](https://tools.ietf.org/html/rfc6570#section-3.2.1, especially since the Guzzle implementation will be removed. However, when I changed from Guzzle's implementation to this one, I think I found a slight bug/incompatibility.

RFC6570 states that:

A variable that is undefined (Section 2.3) has no value and is ignored by the expansion process. If all of the variables in an expression are undefined, then the expression's expansion is the empty string. (emphasis mine)

Therefore, if I wrote the following:

$template = new UriTemplate('{?x,y,undef}');
$params = [
  'x' => 'foo',
  'y' => 42,
];
echo (string) $template->expand($params), PHP_EOL;

I would expect the expanded URI to be:

?x=foo&y=42

... given that the variable undef is not defined in my parameter array.

However, what I actually see is that the URI is expanded to:

?x=foo&y=42&undef=

For comparison, this online URI template tester gives my expected results: https://spooky.github.io/uri-template-tester/

You can try it with:
http://example.org/test{?x,y,undef}

{
  "x": "foo",
  "y": 42
}

You should get http://example.org/test?x=foo&y=42 as shown below.

screenshot of the URI template tester showing the above

@nyamsprod nyamsprod merged commit 3dd2732 into thephpleague:master Feb 29, 2020
@nyamsprod
Copy link
Member

@gabesullice I'll tentatively accept the PR but I will more than probably change some part of the code but the bugfix will be reported as fixed by your contribution thanks for the report and the PR.

nyamsprod added a commit that referenced this pull request Feb 29, 2020
nyamsprod added a commit that referenced this pull request Feb 29, 2020
@gabesullice
Copy link
Contributor Author

Wow, thanks for the quick turnaround @nyamsprod! I assumed the code would change :) it was just the simplest, fastest way to add a test and a fix along with the bug report. Thanks again!

nyamsprod added a commit that referenced this pull request Nov 17, 2022
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.

2 participants