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

Configuration fails for ** when path includes colons #212

Closed
akaralar opened this issue Aug 3, 2017 · 4 comments
Closed

Configuration fails for ** when path includes colons #212

akaralar opened this issue Aug 3, 2017 · 4 comments

Comments

@akaralar
Copy link
Contributor

akaralar commented Aug 3, 2017

In our app, authentication was being set up using service.configure("**") { ... } as was recommended in the user guide.

Fast forward and we implemented push notifications, and the way to delete a firebase token from the backend was to send a DELETE request to /devices/<firebase token string>

The request was failing with authentication errors. I tried to send the request through Paw and it succeeded without errors, so the backend was working fine.

I thought there was a race condition between signing out and deleting tokens but there seemed to be none. Setting up siesta logging to show configurations for requests, I noticed that the ** configuration was being applied to all the requests except the token deletion request. Then I noticed that there was a colon : in the token, and I suspected it might be the cause. I stripped the : in the token string and the configuration was being applied to the request correctly.

I wanted to send a PR for this but I don't know much about regular expressions and it seems the matching is made with those.

@akaralar
Copy link
Contributor Author

akaralar commented Aug 3, 2017

We will probably implement a workaround for this, do DELETE methods allow a json body in siesta?

@pcantrell
Copy link
Member

I noticed that the ** configuration was being applied to all the requests except the token deletion request. Then I noticed that there was a colon : in the token, and I suspected it might be the cause. I stripped the : in the token string and the configuration was being applied to the request correctly.

This is almost certainly a bug in Siesta. I’ll investigate — or you’d be welcome to yourself, if you want to submit a pull request! If you want to work on it, look for the describe("using wilcards") section of ServiceSpec.swift, try adding a test case, and go from there. The actual work happens in `ConfigurationPatternConvertible.swift.

@pcantrell
Copy link
Member

Huh, well, I just dug into this and … I seem to have explicitly treated colons as path separators in the original version of the pattern matching. That’s not right; HTTP path segments can contain colons. What was I thinking? Probably a half-formed thought about host:port matching.

I’ve submitted a fix as #221 (against the swift-4 branch, which is where the action is happening right now). @akaralar, would you mind taking a look?

@akaralar
Copy link
Contributor Author

@pcantrell Sorry I didn't take this up myself. We have some holidays coming up and I might try to see if it works but I'll have to coordinate with our backend developers because they changed that endpoint to receive the data in the body instead of the URL.

Will let you know within a few days. Thanks for the effort!

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