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

Resolve() crashes on whitespace in params bracket #40

Open
khaliddermoumi opened this issue Aug 30, 2016 · 2 comments
Open

Resolve() crashes on whitespace in params bracket #40

khaliddermoumi opened this issue Aug 30, 2016 · 2 comments
Labels
enhancement help wanted This issue is available to submit a pull request for

Comments

@khaliddermoumi
Copy link

When there's a blank in a uri template string, the Resolve()-method crashes.

Meaning:

  • this template will work: "/feeds/events{?fromId}"
  • this template will crash when "Resolve()" is called: "/feeds/events{? fromId}"

I believe the UriTemplates implementation should either moan when constructed with such a template: "/feeds/events{? fromId}", or work correctly. But throwing an exception when "Resolve()" is called is too late.

@darrelmiller
Copy link
Member

I looked into this and I don't have an ideal answer for you. Technically the spec does not allow whitespace where you have it, so by the letter of the law, it is not a valid template.
However, I don't process the template until you actually call resolve, so the error is only detected during processing. In theory I could add in a redundant call to resolve in the constructor, but then everyone pays the price of that extra resolve call.

If you need to know that a template is valid at construction time, then you could call Resolve() immediately after creating the template. There is no harm in calling Resolve() multiple times. Another possibility is to add a Verify() method that returns an error message if it finds anything. That would avoid having to do try catch blocks around the Resolve check.

Thoughts?

@khaliddermoumi
Copy link
Author

khaliddermoumi commented Sep 17, 2016

Ok, this is not a very critical issue, and your proposals make sense.

On the other side, it is a kind of convention that patterns such as URIs get validated at construction time.
Examples:

  • System.Uri constructor throws UriFormatException
  • java.net.URI constructor throws URISyntaxException

I think this behaviour makes sense. I personally would stick to it, I would make the constructor perform a validation.

@baywet baywet added enhancement help wanted This issue is available to submit a pull request for labels Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted This issue is available to submit a pull request for
Projects
None yet
Development

No branches or pull requests

3 participants