[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[hekonsek]

Github user hekonsek closed the pull request at:

https://github.com/apache/qpid-proton/pull/65


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[hekonsek]

Github user hekonsek commented on the pull request:

https://github.com/apache/qpid-proton/pull/65#issuecomment-206744093
  
Done :) .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[astitcher]

Github user astitcher commented on the pull request:

https://github.com/apache/qpid-proton/pull/65#issuecomment-206491900
  
@hekonsek could you close this pull request now, as it is no longer for 
consideration.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[hekonsek]

Github user hekonsek commented on the pull request:

https://github.com/apache/qpid-proton/pull/65#issuecomment-202500451
  
Hi,

Actually as vertx-proton API seems to favored over a messenger API, then 
feel free to ignore this pull request. We will be using vertx-proton anyway.

Cheers!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[ssorj]

Github user ssorj commented on the pull request:

https://github.com/apache/qpid-proton/pull/65#issuecomment-184352363
  
In previous discussion on APIs and bindings, we have said that we'd like to 
use the language-native URL facilities if feasible.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[gemmellr]

Github user gemmellr commented on the pull request:

https://github.com/apache/qpid-proton/pull/65#issuecomment-184320477
  
I hadn't got round to looking at this closely but was likely just going to 
apply it if nothing obvious came up, and probably wouldnt have considered the 
above.

As per the diff, doesnt seem like java Messenger is using the native URL 
bits to parse its URL-like strings, and I don't see anything in there handling 
special char encodings, though I guess it could be hiding elsehwere (I don't 
really know the Messenger bits at all). It does kinda looks like whats there at 
present might accept the topic:// in the path if a scheme is given before it in 
the string, but I haven't tried that.

The idea that the schemes used should be looked for seems sensible, though 
perhaps other values should be rejected if that were to be done, but then it 
wouldn't play nice with without encoding special chars or again having the 
actual scheme present at the start.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[alanconway]

Github user alanconway commented on the pull request:

https://github.com/apache/qpid-proton/pull/65#issuecomment-184307353
  
The problem is that the addresses used by proton are modelled on URLs, and 
those characters are not safe in a URL path:

From RFC 1738 specification:

Thus, only alphanumerics, the special characters "$-_.+!*'(),", and 
reserved characters used for their reserved purposes may be used unencoded 
within a URL.

The charcters ":" and "/" are reserved. Now protons addresses are not 
really standardized anywhere, and we could easily hack on the proton C URL 
parser to do whatever we like. However many of the proton bindings use the 
language-native URL parser instead of the proton C one so breaking the URL 
rules could get into trouble elsewhere.

Note that proton's parser (and any language-binding-native URL parser) 
should respect the standard URL percent-encoding for special characters, so 
that might be the best way to go if you want to encode topic:// as part of a 
URL path.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] qpid-proton pull request: Schema parsing should not be so greedy

[hekonsek]

GitHub user hekonsek opened a pull request:

https://github.com/apache/qpid-proton/pull/65

Schema parsing should not be so greedy

Hi,

Current address schema parser doesn't work if `://` string is included in a 
path. The reasons to include `://` in a path are as follows:
- it is not against AMQP address specification
- ActiveMQ AMQP module uses [1] `topic://`prefix to indicate that message 
should be dispatched to broker topic, not queue (and other broker 
implementations could do the same)

So to connect to ActiveMQ's topic named `topicname`, you could use the 
following AMQP address - `host:423/topic://topicname` . The problem with 
current `Address` implementation is that it will interpret `://` in `topic://` 
as a schema separator.

Since `://` is optional in address and the only two acceptable schemas are 
`amqp` and `ampqs`, I think we should narrow schema parsing to those two values.

What do you think? 

[1] http://activemq.apache.org/amqp.html

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hekonsek/qpid-proton address-schema-parsing

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/qpid-proton/pull/65.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #65


commit 3ac4e79a54ba4b88bcbc8eaedcc281f5fd50d5e9
Author: Henryk Konsek 
Date:   2016-02-05T11:43:39Z

Schema parsing should not be so greedy.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---