Skip to content

s3_auth: accept longer config lines#9090

Merged
vmamidi merged 4 commits intoapache:masterfrom
moonchen:s3-auth-long-config
Sep 21, 2022
Merged

s3_auth: accept longer config lines#9090
vmamidi merged 4 commits intoapache:masterfrom
moonchen:s3-auth-long-config

Conversation

@moonchen
Copy link
Copy Markdown
Contributor

  • Allow arbitrarily long config lines in s3_auth config files. Previously the limit was 511 characters.

    The size of the security token that AWS STS API operations return is not fixed. We strongly recommend that you make no assumptions about the maximum size.

  • Add autest for config parsing.

@moonchen moonchen marked this pull request as draft September 13, 2022 15:58
@moonchen moonchen marked this pull request as ready for review September 13, 2022 16:13
Rename v4-parse-test.conf to .test_input
@bryancall bryancall added this to the 10.0.0 milestone Sep 13, 2022
@bryancall bryancall requested a review from bneradt September 19, 2022 23:13
Copy link
Copy Markdown
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Makes sense.

Comment thread plugins/s3_auth/s3_auth.cc Outdated
Comment thread plugins/s3_auth/aws_auth_v4.cc Outdated
Comment thread plugins/s3_auth/s3_auth.cc Outdated
Comment thread plugins/s3_auth/aws_auth_v4.h Outdated
Comment thread tests/gold_tests/pluginTest/s3_auth/s3_auth_config.test.py Outdated
Comment thread tests/gold_tests/pluginTest/s3_auth/s3_auth_config.test.py Outdated
Comment thread tests/gold_tests/pluginTest/s3_auth/s3_auth_config.test.py Outdated
@moonchen moonchen requested a review from bneradt September 20, 2022 20:01
Comment thread tests/gold_tests/pluginTest/s3_auth/s3_auth_config.test.py Outdated
@bneradt
Copy link
Copy Markdown
Contributor

bneradt commented Sep 20, 2022

Setting for 9.2.x per @moonchen 's request. The change should be pretty safe and he added quite a bit of testing for the parts he changed. And it sounds like Apple will be running with this internally as well so if there are issues they'll most likely find it there.

@vmamidi vmamidi merged commit cd712e0 into apache:master Sep 21, 2022
zwoop pushed a commit that referenced this pull request Oct 4, 2022
* s3_auth: accept longer config lines

* Tell CI to ignore intentional trailing spaces

Rename v4-parse-test.conf to .test_input

* Address PR review comments

* Remove spurious os import

(cherry picked from commit cd712e0)
@zwoop
Copy link
Copy Markdown
Contributor

zwoop commented Oct 4, 2022

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Oct 4, 2022
Ftywan pushed a commit to Ftywan/trafficserver that referenced this pull request Dec 21, 2022
* s3_auth: accept longer config lines

* Tell CI to ignore intentional trailing spaces

Rename v4-parse-test.conf to .test_input

* Address PR review comments

* Remove spurious os import

Co-authored-by: Mo Chen <uncorrupt@gmail.com>
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
* asf/9.2.x:
  Updated ChangeLog
  s3_auth: Fix parsing of virtual_host (apache#9103)
  s3_auth: accept longer config lines (apache#9090)
  Add a proxy.config.http.per_server.connection.max test (apache#9097)
  Move plugin_init for verify cmd (apache#9102)
  Fix unused-but-set-variable warnings by llvm-15 (apache#9106)
  Fix compile error with llvm-15 (apache#9105)
  Fix expected sha1sum for the Proxy Verifier binary (apache#9112)
  Updating AuTest to use Proxy Verifier v2.4.2 (apache#9110)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants