I *think* https://cs.chromium.org/chromium/src/content/browser/web_package/signed_exchange_cert_fetcher.cc?l=172&rcl=725e8cdec53eb9f5470d0105cb6ab58ee265380f requires certUrl to be absolute, but https://cs.chromium.org/chromium/src/content/browser/web_package/signed_exchange_header_parser.cc?l=230&rcl=dc1f1329b5b2560b460b6243121dc35f36a8d571 doesn't check it explicitly. I've written down this requirement in https://github.com/WICG/webpackage/pull/170.
We already have the GURL.is_valid() check for certUrl and validityUrl. But we don't have the check for request url. https://chromium.googlesource.com/chromium/src/+/8ba3e153b566e20156093dd96fc1134ff86b9640/content/browser/web_package/signed_exchange_header.cc#84 And also there is no check of the reference fragment (#foobar) for all of them.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da1e9ae5f918c66f3f35f56b00190f5ed9773405 commit da1e9ae5f918c66f3f35f56b00190f5ed9773405 Author: Tsuyoshi Horo <horo@chromium.org> Date: Tue Apr 10 07:43:22 2018 Check the validity of URLs of SignedHTTPExchange CertUrl and validityUrl and request URL must be absolute. https://github.com/WICG/webpackage/pull/170 Currently the validity of certUrl and validityUrl is checked in ParseSignature(). But there is no validity check of the request url. And also there is no has_ref() check of all of them. So this CL adds these checks: - is_valid() and !has_ref() for request URL. - !has_ref() check for certUrl and validityUrl. Bug: 829932 Change-Id: I4502cb7cbb381e631292c8fd7a1ca998614f21b3 Reviewed-on: https://chromium-review.googlesource.com/1002338 Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#549446} [modify] https://crrev.com/da1e9ae5f918c66f3f35f56b00190f5ed9773405/content/browser/web_package/signed_exchange_header.cc [modify] https://crrev.com/da1e9ae5f918c66f3f35f56b00190f5ed9773405/content/browser/web_package/signed_exchange_header_parser.cc [modify] https://crrev.com/da1e9ae5f918c66f3f35f56b00190f5ed9773405/content/browser/web_package/signed_exchange_header_parser_unittest.cc [modify] https://crrev.com/da1e9ae5f918c66f3f35f56b00190f5ed9773405/content/browser/web_package/signed_exchange_header_unittest.cc
Thanks! Could you add a test that a relative URL is rejected? I don't see documentation that GURL::is_valid() returns false for relative URLs, which was the original question in https://github.com/WICG/webpackage/issues/147.
Sure. Created http://crrev.com/c/1004836
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd9526165a87f22c2ef6bc26d1b671684e7315a2 commit bd9526165a87f22c2ef6bc26d1b671684e7315a2 Author: Tsuyoshi Horo <horo@chromium.org> Date: Wed Apr 11 04:54:15 2018 Add tests to check that relative URL is rejected in SignedHTTPExchange Bug: 829932 Change-Id: Ic437dd2cc43ccbfbf83a100d1c6a383261cddc6b Reviewed-on: https://chromium-review.googlesource.com/1004836 Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#549766} [modify] https://crrev.com/bd9526165a87f22c2ef6bc26d1b671684e7315a2/content/browser/web_package/signed_exchange_header_parser_unittest.cc [modify] https://crrev.com/bd9526165a87f22c2ef6bc26d1b671684e7315a2/content/browser/web_package/signed_exchange_header_unittest.cc
Comment 1 by horo@chromium.org
, Apr 9 2018