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

Allow path chunks to start with a colon #128

Merged
merged 1 commit into from Mar 7, 2016

Conversation

jstepien
Copy link
Contributor

@jstepien jstepien commented Mar 3, 2016

Paths with chunks starting with colons — /like/::this/:one — are valid
but they were not parsed correctly by bidi. The reason was an exception
thrown by the java.net.URI constructor invoked in bidi/just-path.
java.net.URI assumed that a string like ":a" was missing a required URI
schema and threw. Similar issues occurred in the ClojureScript edition.

This patch solves the problem by prefixing the path with an explicit
schema file:/// before invoking the constructor. Afterwards it drops
the slash which got prepended.

Interestingly enough, replacing bidi/just-path with the following
implementation solves the problem too and according to the test suite
it doesn't appear to be introducing any regressions. This being said,
my gut feeling tells me it might bite us in the future.

(defn just-path
  [path]
  (apply str (take-while #(not= \? %) path))

Paths with chunks starting with colons — /like/::this/:one — are valid
but they were not parsed correctly by bidi. The reason was an exception
thrown by the java.net.URI constructor invoked in bidi/just-path.
java.net.URI assumed that a string like ":a" was missing a required URI
schema and threw.  Similar issues occurred in the ClojureScript edition.

This patch solves the problem by prefixing the path with an explicit
schema file:/// before invoking the constructor. Afterwards it drops
the slash which got prepended.

Interestingly enough, replacing bidi/just-path with the following
implementation solves the problem too and according to the test suite
it doesn't appear to be introducing any regressions. This being said,
my gut feeling tells me it might bite us in the future.

   (defn just-path
     [path]
     (apply str (take-while #(not= \? %) path))
malcolmsparks added a commit that referenced this pull request Mar 7, 2016
Allow path chunks to start with a colon
@malcolmsparks malcolmsparks merged commit 40aa779 into juxt:master Mar 7, 2016
@malcolmsparks
Copy link
Contributor

Looks good to me. Thanks.

@jstepien jstepien deleted the colons branch March 7, 2016 13:45
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.

None yet

2 participants