You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/05/11 00:11:25 UTC

[GitHub] [trafficserver] masaori335 opened a new pull request #6753: Fix HPACK Dynamic Table Cleanup

masaori335 opened a new pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753


   Fix #6748. This is introduced by TS-3719, it was trying to fix a bug.
   
   > The table size is reduced by the by max size - new size instead of current size - new size. This causes the table to try to delete items that don't exist.
   
   We don't need to worry about this with the current code, because we're using a for loop now.
   
   https://github.com/apache/trafficserver/blob/4638ddcc2848da53b94f8678d22057d8efda8df3/proxy/http2/HPACK.cc#L427
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] masaori335 edited a comment on pull request #6753: Fix HPACK Dynamic Table Cleanup

Posted by GitBox <gi...@apache.org>.
masaori335 edited a comment on pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753#issuecomment-630611858


   All right, I broke the feature by 206384e with misunderstanding what the check is doing. IMO, we need more tests to cover edge cases like this PR has.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] zwoop commented on pull request #6753: Fix HPACK Dynamic Table Cleanup

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753#issuecomment-631546963


   Cherry-picked to 8.1.x


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] masaori335 merged pull request #6753: Fix HPACK Dynamic Table Cleanup

Posted by GitBox <gi...@apache.org>.
masaori335 merged pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] zwoop commented on pull request #6753: Fix HPACK Dynamic Table Cleanup

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753#issuecomment-631546455


   Cherry-picked to v9.0.x branch.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] masaori335 commented on pull request #6753: Fix HPACK Dynamic Table Cleanup

Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753#issuecomment-630611858


   IMO, we need more tests to cover edge cases like this PR has.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] maskit commented on pull request #6753: Fix HPACK Dynamic Table Cleanup

Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #6753:
URL: https://github.com/apache/trafficserver/pull/6753#issuecomment-628291418


   I agree that 96bd1fa fixed a security issue, but I'd say the fix was not the way it should. As long as the eviction logic is correct, the function doesn't cause a compression error.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org