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 an issue with the servlet mapping for JSR 356 #2331

Merged
merged 1 commit into from
May 7, 2018
Merged

Resolve an issue with the servlet mapping for JSR 356 #2331

merged 1 commit into from
May 7, 2018

Conversation

fenik17
Copy link
Contributor

@fenik17 fenik17 commented May 6, 2018

After some digging about #2316 (and #2326) I came to a conclusion that servlet mapping with trailing slash is incorrect in JSR 356 [1]. The mapping value must have a leading '/' and should not contain empty parts:

The value attribute must be a Java string that is a partial URI or URI-template (level-1), with a leading
‘/’. For a definition of URI-templates, see [6]. The implementation uses the value attribute to deploy the
endpoint to the URI space of the websocket implementation. The implementation must treat the value as relative to the root URI of the websocket implementation in determining a match against the request URI of an incoming opening handshake request. [WSC-4.1.1-2] The semantics of matching for annotated endpoints is the same as was defined in the previous chapter. The value attribute is mandatory; the implementation must reject a missing or malformed path at deployment time [WSC-4.1.1-3].

The trailing slash can be ignored or treated as missed part, as we saw in a Tomcat's implementation (#2320). Up to #2316 everything worked correctly: the /mapping/ was cleared on /mapping and were not registering templates with trailing slash.

This PR:

[1] https://jcp.org/aboutJava/communityprocess/mrel/jsr356/index.html
[2] https://www.w3.org/Addressing/URL/url-spec.txt

- Revert #2316 as incorrect. JSR 356 is not allowed empty part in path.
- Fix regexp for the servlet path cleaning: remove an unclosed character
  class and allow '+' as part of path.
- Add more test cases for the fixed regexp.
@fenik17
Copy link
Contributor Author

fenik17 commented May 6, 2018

@Artur- PTAL. Maybe I'm wrong?

@fenik17 fenik17 self-assigned this May 6, 2018
@fenik17 fenik17 requested a review from jfarcand May 6, 2018 21:27
@fenik17 fenik17 added this to the 4.2.25 milestone May 6, 2018
@jfarcand
Copy link
Member

jfarcand commented May 7, 2018

See #2202

@jfarcand jfarcand merged commit 2e8f3e9 into Atmosphere:master May 7, 2018
@fenik17 fenik17 deleted the jsr356_mapping_revert branch May 7, 2018 19:45
@Artur-
Copy link
Contributor

Artur- commented May 8, 2018

Sorry for the lack of response. I do not think the regexp helps as it is about finding the servlet path but my problem was in what happens after that

@fenik17
Copy link
Contributor Author

fenik17 commented May 8, 2018

@Artur- yep, regexp do not affect for your issue. Can you describe the core problem that you are trying to solve?

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.

3 participants