You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by hekonsek <gi...@git.apache.org> on 2016/02/05 12:55:57 UTC
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
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 <he...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by ssorj <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by astitcher <gi...@git.apache.org>.
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/65#issuecomment-184349901
I think that @alanconway has a good point.
Looking at the C implementation of the url decoder it doesn't seem to expand the % escapes for the path, but as far as I can see it should - it would be simple to make it do that.
I would say that the Java and C url parsers should be closely aligned (even if they can't be exactly the same code)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by hekonsek <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by hekonsek <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by hekonsek <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by alanconway <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by gemmellr <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: Schema parsing should not be so greedy
Posted by astitcher <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---