Skip to content

Conversation

faridtsl
Copy link
Contributor

@faridtsl faridtsl commented Feb 1, 2021

This change adds the header Content-Length to HEAD HTTP requests.

The previous behavior was blocking some Windows executables (i.e
bitsadmin.exe) from downloading files hosted in Gitea.

This along with PR #14541, makes the web server compliant with HTTP RFC 2616 which states
"The methods GET and HEAD MUST be supported by all general-purpose servers"
and
"The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response."

This should also respond to issues #8030 and #14532.

This change adds the header Content-Length to HEAD HTTP requests.

The previous behaviour was blocking some Windows executables (i.e
bitsadmin.exe) from downloading files hosted in Gitea.

This along with PR go-gitea#14541, makes the web server compliant with HTTP RFC 2616 which states
"The methods GET and HEAD MUST be supported by all general-purpose servers"
and
"The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response."

This should also respond to issues go-gitea#8030 and go-gitea#14532.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 1, 2021
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

If this really represents a unbounded read this is tantamount to a security risk.

Pass the Size of the content as a parameter to ServeData() instead of
calculating it using ioutil.ReadAll(reader) --> this call is dangerous
and can result in a denial of service.
@faridtsl faridtsl requested a review from zeripath February 1, 2021 21:42
Quick fix for imported dependency not used.
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 1, 2021
@6543 6543 added the type/enhancement An improvement of existing functionality label Feb 2, 2021
@6543 6543 added this to the 1.14.0 milestone Feb 2, 2021
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

LGTM except https://github.com/go-gitea/gitea/pull/14542/files#r568258348, I think we should add the head only size > 0.

@zeripath How did you think this maybe a security problem?

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 2, 2021
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

Much better! Thanks!

@6543 6543 merged commit f72ce26 into go-gitea:master Feb 5, 2021
a1012112796 added a commit to a1012112796/gitea that referenced this pull request Feb 9, 2021
* master: (22 commits)
  Add support for ref parameter to get raw file API (go-gitea#14602)
  Fixed irritating error message related to go version (go-gitea#14611)
  Use OldRef instead of CommitSHA for DeleteBranch comments (go-gitea#14604)
  Add information on how to build statically (go-gitea#14594)
  [skip ci] Updated translations via Crowdin
  Exclude the current dump file from the dump (go-gitea#14606)
  Remove spurious DataAsync Error logging (go-gitea#14599)
  [API] Add  delete release by tag & fix unreleased inconsistency (go-gitea#14563)
  Fix rate limit bug when downloading assets on migrating from github (go-gitea#14564)
  [API] Add affected files of commits to commit struct (go-gitea#14579)
  [skip ci] Updated licenses and gitignores
  Fix locale init (go-gitea#14582)
  Add Content-Length header to HEAD requests (go-gitea#14542)
  Honor REGISTER_MANUAL_CONFIRM when doing openid registration (go-gitea#14548)
  Fix lfs file viewer (go-gitea#14568)
  Fix typo in generate-emoji.go (go-gitea#14570)
  Fix bug about ListOptions and stars/watchers pagnation (go-gitea#14556)
  Fix gpg key deletion (go-gitea#14561)
  [API] GetRelease by tag only return release (go-gitea#14397)
  Reduce data races (go-gitea#14549)
  ...
@go-gitea go-gitea locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants