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 2021/05/26 18:06:42 UTC
[GitHub] [trafficserver] bneradt opened a new pull request #7882: Add an HTTP/2 304 "Not Modified" AuTest.
bneradt opened a new pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882
This adds an HTTP/2 304 "Not Modified" test to verify that we handle it
correctly.
This fixes #3147
--
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 #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882#issuecomment-861982682
Use of Proxy Verifier was suggested. Is it possible to test the case with curl now?
--
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] bneradt commented on pull request #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882#issuecomment-861073911
> * Should we add a range request that updates a stale object?
> * with h2 expected to have more requests per stream, should we test multiple requests?
> * Should we add any cases from https://cache-tests.fyi/#update304 ?
These are good ideas. Thank you. For this PR, however, I'd like to stick to just this more narrow coverage. This PR is made to address a particular test need described in #3147 which couldn't be satisfied at the time due to a deficiency in curl's HTTP/2 read behavior.
--
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] bneradt commented on pull request #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882#issuecomment-863532249
> Looks good to me, but I'm not sure if we can close #3147. The motivation of #3147 seems like to be testing 304 response with an empty body.
The 304 from ATS has an empty body.
> Use of Proxy Verifier was suggested. Is it possible to test the case with curl now?
Yes, Proxy Verifier was suggested because curl wasn't dealing with the end of the stream appropriately. But it seems to be now. When I add debug to the curl I'm using, I see that the on_stream_close callback is appropriately called.
--
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] mlibbey commented on pull request #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
mlibbey commented on pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882#issuecomment-849152299
- Should we add a range request that updates a stale object?
- with h2 expected to have more requests per stream, should we test multiple requests?
- Should we add any cases from https://cache-tests.fyi/#update304 ?
--
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] bneradt commented on pull request #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882#issuecomment-861072483
> Having more tests is a good thing so I'm not going to block this, however, I wouldn't put the test under h2 directory because 304 is not an H2 specific thing. I wrote this on some previous PR before, but I think we should put tests under h2 directory only if those check H2 specific behavior. Other tests should be under other directories accordingly and the tests should be ran on all the HTTP versions.
Sounds good. I've moved the test to under cachedIMSRange.test.py which has other 304 tests with it already. I've also added HTTP and HTTPS versions of this test as well.
Thanks for the review.
--
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 #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882#issuecomment-849204071
Having more tests is a good thing so I'm not going to block this, however, I wouldn't put the test under h2 directory because 304 is not an H2 specific thing. I wrote this on some previous PR before, but I think we should put tests under h2 directory only if those check H2 specific behavior. Other tests should be under other directories accordingly and the tests should be ran on all the HTTP versions.
--
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] bneradt merged pull request #7882: Add an HTTP/2 304 "Not Modified" AuTest.
Posted by GitBox <gi...@apache.org>.
bneradt merged pull request #7882:
URL: https://github.com/apache/trafficserver/pull/7882
--
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