jakartaee / websocket

Jakarta WebSocket
https://projects.eclipse.org/projects/ee4j.websocket
Other
61 stars 43 forks source link

Clarify what is a valid WebSocket path spec variable name #193

Closed glassfishrobot closed 4 years ago

glassfishrobot commented 11 years ago

As brought up in jsr356-experts: http://java.net/projects/websocket-spec/lists/jsr356-experts/archive/2013-04/message/6

What makes up a valid websocket path spec variable name?

Some examples of bad path specs with variable names present:

1) "/a/{var/b}" - path segment separator in variable name 2) "/a/{var*}" - use of glob character not allowed in variable name. 3) "/a/{}" - no variable name declared 4) "/a/{---}" - no alpha variable name declared 5) "/a/{my special variable}" - spaces not allowed

Example 1, it could be allowed as a funky variable named "var/b", but is this something we should alert the user of as a possible typo?

Example 2, it could also be allowed as a funky variable named "var*", again, this should probably be alerted to the user as bad form.

Example 3 has no variable name, it should just be forbidden

Example 4, it could also be a funky variable named "---".

Should we restrict the allowable variable names to conform to some sort of limited scope of characters? Say regex "[a-zA-Z0-9._-]*" ?

It was brought up that RFC 6570 already provides some suitable limits for variable names.

Looks like Section 2.3. Variables has the details. http://tools.ietf.org/html/rfc6570#section-2.3

Looks like it would allow for pct-encoded (Section 1.5) in varname. Sounds reasonable.

To translate what I see. varchar = (ALPHA / DIGIT / "_" / pct-encoded) varname = varchar *( ["."] varchar)

So that would make the following Good Variable names.

  {a}
  {ab}
  {a.b}
  {a_b}
  {alpha.beta}
  {Answer_42}
  {_}
  {1_000_000}
  {%40ClientEndpoint}

and the following would be Bad Variable names.

  {a b}
  {a..b}
  {alpha }
  { beta}
  {1,000,000}

The direction of RFC 6570 seems like a good start, but some of the things it seems to allow might complicate things.

glassfishrobot commented 6 years ago
glassfishrobot commented 11 years ago

@glassfishrobot Commented Reported by joakimerdfelt

glassfishrobot commented 11 years ago

@glassfishrobot Commented markt_asf said: The limits from RFC6570 seem reasonable to me. Which ones do you think could be problematic?

glassfishrobot commented 11 years ago

@glassfishrobot Commented dannycoward said: Hey there,

We are using level-1 templates only (e.g. see the spec document section 4.1.1). RFC6570 makes an unambiguous definition of what the legal characters are there.

Let me know if there are any ambiguities in the RFC6570 definition - Danny

glassfishrobot commented 11 years ago

@glassfishrobot Commented joakimerdfelt said: RFC6570 has only 1 ambiguous part with how it relates to our JSR.

Section 2.3 - Variables http://tools.ietf.org/html/rfc6570#section-2.3

variable-list = varspec ( "," varspec ) varspec = varname [ modifier-level4 ] varname = varchar ( ["."] varchar ) varchar = ALPHA / DIGIT / "_" / pct-encoded

So that means, to the JSR, that @ServerEndpoint("/fruit/%40Type") would be allowed. (My reading of level-1 variable names would be what 'varname' is defined like above) However, when the @PathParam matching it up to the variable is concerned, do we follow the same 'varname' syntax as well? (this is not specified in section 4.3 of our JSR) The declaration could be either @PathParam("%40Type") (leaving 'pct-encoded' alone) or @PathParam("@Type") (decoded form of the pct-encoded)

glassfishrobot commented 7 years ago

@glassfishrobot Commented This issue was imported from java.net JIRA WEBSOCKET_SPEC-193

markt-asf commented 4 years ago

I don't believe the ambiguity described in the previous comment is valid. The annotation would be @ServerEndpoint("/fruit/{%40Type}") As per RFC 6570:

A varname containing pct-encoded characters is not the same variable as a varname with those same characters decoded.

Therefore, the path parameter definition would have to be @PathParam("%40Type").

I am therefore closing this issue.