Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/otlpreceiver] improve request's content-type check #7451

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

macno
Copy link
Contributor

@macno macno commented Mar 29, 2023

content-type header could contain optional parameters after the mime-type, do not reject requests in this case

note: this is my first PR in this repo, excuse me if I miss something and guide me on moving forward, thanks!

Description:

If the HTTP request contains a valid content-type header in the form application/json; charset=utf-8 the receiver replies with 415 unsupported media type, supported: .
Also, by spec, header is case-insensitive, so we take care also of this.

Link to tracking Issue:

Fixes #7452

Testing:
Added unit tests to verify the bug and see that the change fixes it

Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

This will also need a changelog entry, there's a guide on how to add one here.

receiver/otlpreceiver/otlp_test.go Show resolved Hide resolved
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for fixing this.

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (d4c25d4) 91.11% compared to head (178ef18) 91.11%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7451   +/-   ##
=======================================
  Coverage   91.11%   91.11%           
=======================================
  Files         292      292           
  Lines       14205    14210    +5     
=======================================
+ Hits        12943    12948    +5     
  Misses        996      996           
  Partials      266      266           
Impacted Files Coverage Δ
receiver/otlpreceiver/otlp.go 82.94% <100.00%> (ø)
receiver/otlpreceiver/otlphttp.go 47.78% <100.00%> (+2.41%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

content-type header could contain optional parameters after the mime-type,
do not reject requests in this case
@codeboten codeboten merged commit e895d0d into open-telemetry:main Apr 27, 2023
@github-actions github-actions bot added this to the next release milestone Apr 27, 2023
codeboten pushed a commit to codeboten/opentelemetry-collector that referenced this pull request May 2, 2023
…lemetry#7451)

content-type header could contain optional parameters after the mime-type,
do not reject requests in this case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Code review completed; ready to merge by maintainers
Projects
None yet
4 participants