Skip to content

Don't auto-send STOMP content-length header if one was explicitly set#86

Closed
scop wants to merge 1 commit intoapache:masterfrom
scop:dupecontentlength
Closed

Don't auto-send STOMP content-length header if one was explicitly set#86
scop wants to merge 1 commit intoapache:masterfrom
scop:dupecontentlength

Conversation

@scop
Copy link
Copy Markdown
Contributor

@scop scop commented Jul 20, 2015

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this introduce any form of a security issue? What happens on the receiver side if the content length doesn't match the header?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@clebertsuconic
Copy link
Copy Markdown
Contributor

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.

@clebertsuconic
Copy link
Copy Markdown
Contributor

@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 :)

@asfgit asfgit closed this in fbfdacd Jul 20, 2015
@scop scop deleted the dupecontentlength branch July 20, 2015 14:53
@johnament
Copy link
Copy Markdown
Contributor

@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.

d0k1 pushed a commit to d0k1/activemq-artemis that referenced this pull request Dec 21, 2015
andytaylor added a commit to andytaylor/artemis that referenced this pull request Jul 26, 2016
ARTEMIS-459 NPE during RA tearDown
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