You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by scop <gi...@git.apache.org> on 2015/07/20 11:04:16 UTC
[GitHub] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
GitHub user scop opened a pull request:
https://github.com/apache/activemq-artemis/pull/86
Don't auto-send STOMP content-length header if one was explicitly set
I'm not sure if a code path where this (= sending more than one content-length header) happens exists, but nothing prevents the issue from happening ATM.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/scop/activemq-artemis dupecontentlength
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/86.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 #86
----
commit fc1bebb0ad24629d94a0ef56ff56dd3f1567248b
Author: Ville Skyttä <vi...@iki.fi>
Date: 2015-07-20T09:01:46Z
Don't auto-send STOMP content-length header if one was explicitly set
----
---
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] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/86
---
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] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
Posted by scop <gi...@git.apache.org>.
Github user scop commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/86#discussion_r34989913
--- Diff: artemis-protocols/artemis-stomp-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/v11/StompFrameV11.java ---
@@ -66,7 +66,7 @@ public ActiveMQBuffer toActiveMQBuffer() throws Exception
head.append(h.getEncodedValue());
head.append(Stomp.NEWLINE);
}
- if (bytesBody != null && bytesBody.length > 0)
+ if (bytesBody != null && bytesBody.length > 0 && !hasHeader(Stomp.Headers.CONTENT_LENGTH))
--- End diff --
I don't think there's any way to know. But then again there probably isn't any way to know what might happen on client side if there are duplicate or multiple content-length headers.
---
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] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on the pull request:
https://github.com/apache/activemq-artemis/pull/86#issuecomment-122944335
@clebertsuconic none taken at all.
I just know that issues like this tend to lead to buffer overflow type errors. It's technically an unrelated issue, I'll take a look at the server side.
---
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] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/86#discussion_r34987573
--- Diff: artemis-protocols/artemis-stomp-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/v11/StompFrameV11.java ---
@@ -66,7 +66,7 @@ public ActiveMQBuffer toActiveMQBuffer() throws Exception
head.append(h.getEncodedValue());
head.append(Stomp.NEWLINE);
}
- if (bytesBody != null && bytesBody.length > 0)
+ if (bytesBody != null && bytesBody.length > 0 && !hasHeader(Stomp.Headers.CONTENT_LENGTH))
--- End diff --
Does this introduce any form of a security issue? What happens on the receiver side if the content length doesn't match the header?
---
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] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/86#issuecomment-122896188
I think this is ok.. besides.. I need to solve a little conflict with my other change.. so I will merge this now.
@johnament if you still think there's something wrong we can still review this (commit-then-review).. The ideas on these PRs is just to avoid mistakes not to censor commits, we can still rollback later if there's an issue with it.
---
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] activemq-artemis pull request: Don't auto-send STOMP content-lengt...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/86#issuecomment-122896523
@johnament I'm not trying to prevent any discussions here though.. please don't take me wrong.. I'm just saying I will go ahead and merge this.. don't read anything on it please :)
---
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.
---