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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SetAllowGetMethodPayload option ignored for io.Reader payload #541

Merged

Conversation

picollomartin
Copy link
Contributor

Hi 馃憢 !

We detect some inconsistent behaviour of the current implementation of the SetAllowGetMethodPayload based on the input type passed to SetBody option:

  • SetAllowGetMethodPayload = false

    • Body type = io.Reader => body is sent in GET
    • Body type = non io.Reader => body is not sent in GET
  • SetAllowGetMethodPayload = true

    • Body type = io.Reader => body is sent in GET
    • Body type = non io.Reader => body is sent in GET

Basically you can bypass the option AllowGetMethodPayload if you send an io.Reader as a Body, this fixes that for be consistent to all types and add a test case for test disabled option

@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

Merging #541 (0f75cb5) into master (45d9f8b) will decrease coverage by 0.55%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #541      +/-   ##
==========================================
- Coverage   96.39%   95.84%   -0.55%     
==========================================
  Files          10       10              
  Lines        1330     1325       -5     
==========================================
- Hits         1282     1270      -12     
- Misses         30       34       +4     
- Partials       18       21       +3     
Impacted Files Coverage 螖
middleware.go 91.27% <100.00%> (-1.59%) 猬囷笍
request.go 95.25% <0.00%> (-1.10%) 猬囷笍

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

@jeevatkm jeevatkm force-pushed the fix-allow-get-method-payload branch from 7eb3875 to 3546a3b Compare March 6, 2023 00:24
@jeevatkm
Copy link
Member

jeevatkm commented Mar 6, 2023

@picollomartin I'm sorry for the delayed attention on the PR. Thank you for your contribution.
Agreed, inconsistent behavior will lead to consuming investigation time. This PR provided insight for #618.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants