You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2015/08/06 20:58:24 UTC

Review Request 37195: Validate the incoming frame header

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37195/
-----------------------------------------------------------

Review request for qpid and Robbie Gemmell.


Bugs: proton-976
    https://issues.apache.org/jira/browse/proton-976


Repository: qpid-proton-git


Description
-------

Checks the DOFF value in the frame header, and compares the frame size against the transport's max frame limit


Diffs
-----

  proton-c/src/dispatcher/dispatcher.c de6e1f9 
  proton-c/src/framing/framing.h d9fd550 
  proton-c/src/framing/framing.c dde6e6f 
  proton-c/src/proton-dump.c 520299c 
  tests/python/proton_tests/transport.py 07268e1 

Diff: https://reviews.apache.org/r/37195/diff/


Testing
-------

New unit tests added


Thanks,

Kenneth Giusti


Re: Review Request 37195: Validate the incoming frame header

Posted by Kenneth Giusti <kg...@apache.org>.

> On Aug. 7, 2015, 11:44 a.m., Robbie Gemmell wrote:
> > Looks good. I verified that this stopped the crashes seen when trying to use DIGEST-MD5. I even saw (viatrace logs) the example work entirely and send its message some times.
> > 
> > I did have some tiny tweaks for the tests, to use a legal DOFF for the frame size test and fix its description:
> > http://paste.apache.org/7fbX
> > 
> > I also updated proton-j (based on your mailed patch) to validate the frame size as well (as ntoed, it already handled the DOFF) and remove the related test skip:
> > https://paste.apache.org/5pKV

Awesome - thanks so much Robbie.   I'll apply those patches, test and submit.


- Kenneth


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37195/#review94529
-----------------------------------------------------------


On Aug. 6, 2015, 6:58 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37195/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 6:58 p.m.)
> 
> 
> Review request for qpid and Robbie Gemmell.
> 
> 
> Bugs: proton-976
>     https://issues.apache.org/jira/browse/proton-976
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> Checks the DOFF value in the frame header, and compares the frame size against the transport's max frame limit
> 
> 
> Diffs
> -----
> 
>   proton-c/src/dispatcher/dispatcher.c de6e1f9 
>   proton-c/src/framing/framing.h d9fd550 
>   proton-c/src/framing/framing.c dde6e6f 
>   proton-c/src/proton-dump.c 520299c 
>   tests/python/proton_tests/transport.py 07268e1 
> 
> Diff: https://reviews.apache.org/r/37195/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Re: Review Request 37195: Validate the incoming frame header

Posted by Robbie Gemmell <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37195/#review94529
-----------------------------------------------------------

Ship it!


Looks good. I verified that this stopped the crashes seen when trying to use DIGEST-MD5. I even saw (viatrace logs) the example work entirely and send its message some times.

I did have some tiny tweaks for the tests, to use a legal DOFF for the frame size test and fix its description:
http://paste.apache.org/7fbX

I also updated proton-j (based on your mailed patch) to validate the frame size as well (as ntoed, it already handled the DOFF) and remove the related test skip:
https://paste.apache.org/5pKV

- Robbie Gemmell


On Aug. 6, 2015, 6:58 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37195/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 6:58 p.m.)
> 
> 
> Review request for qpid and Robbie Gemmell.
> 
> 
> Bugs: proton-976
>     https://issues.apache.org/jira/browse/proton-976
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> Checks the DOFF value in the frame header, and compares the frame size against the transport's max frame limit
> 
> 
> Diffs
> -----
> 
>   proton-c/src/dispatcher/dispatcher.c de6e1f9 
>   proton-c/src/framing/framing.h d9fd550 
>   proton-c/src/framing/framing.c dde6e6f 
>   proton-c/src/proton-dump.c 520299c 
>   tests/python/proton_tests/transport.py 07268e1 
> 
> Diff: https://reviews.apache.org/r/37195/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Re: Review Request 37195: Validate the incoming frame header

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37195/#review94505
-----------------------------------------------------------

Ship it!


Ship It!

- Gordon Sim


On Aug. 6, 2015, 6:58 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37195/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 6:58 p.m.)
> 
> 
> Review request for qpid and Robbie Gemmell.
> 
> 
> Bugs: proton-976
>     https://issues.apache.org/jira/browse/proton-976
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> Checks the DOFF value in the frame header, and compares the frame size against the transport's max frame limit
> 
> 
> Diffs
> -----
> 
>   proton-c/src/dispatcher/dispatcher.c de6e1f9 
>   proton-c/src/framing/framing.h d9fd550 
>   proton-c/src/framing/framing.c dde6e6f 
>   proton-c/src/proton-dump.c 520299c 
>   tests/python/proton_tests/transport.py 07268e1 
> 
> Diff: https://reviews.apache.org/r/37195/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>