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

Working on PR for "Missing HEAD method when the code moved to chi" #14532

Closed
1 of 2 tasks
faridtsl opened this issue Jan 30, 2021 · 1 comment
Closed
1 of 2 tasks

Working on PR for "Missing HEAD method when the code moved to chi" #14532

faridtsl opened this issue Jan 30, 2021 · 1 comment
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail

Comments

@faridtsl
Copy link
Contributor

-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
/data/testgitea(master*) # curl --head https://try.gitea.io/anyuser/anyrepo/raw/branch/master/anyfile.png                                                                                                                                                                                                                                                                                                                                        
HTTP/2 405
date: Sat, 30 Jan 2021 23:04:16 GMT
set-cookie: i_like_gitea=10985d2770de63e2; Path=/; HttpOnly
set-cookie: _csrf=kFz_ot6_LVT-Jsf6zgWksSWXVLU6MTYxMjA0Nzg1NjQ0NDU5MjM5MA; Path=/; Expires=Sun, 31 Jan 2021 23:04:16 GMT; HttpOnly
set-cookie: macaron_flash=; Path=/; Max-Age=0; HttpOnly
x-frame-options: SAMEORIGIN

Description

When I was trying to fix issue #8030 I had a look at the code base. Then two days later it moved from Macaron to Chi, the HEAD HTTP method no longer works automatically when GET is defined.

I already added and tested the code locally. Adding this line to WebRoutes function in 'routers/routes/web.go' fixes the issue.

r.Use(middleware.GetHead)

As for fixing issue #8030 :

This is a problem when trying to make a program download from Gitea. For example, Windows executables break when trying to download from Gitea /raw route as it does not give "Content-Length" in HEAD response.

The only function that needed to change is ServeData in 'routers/repo/download.go'. By changing the call to io.Copy to ioutil.ReadAll() + ctx.Resp.Write() . Then Writing the Content-Length explicitly by taking the length of the ioutil.ReadAll() byte array.

Questions :
1- Are the design decisions good ? should I make a PR ?
2- Should I make one or two PRs ?
3- I have no idea how to write tests these two functionalities ?

Related issues

#8030

@lunny
Copy link
Member

lunny commented Feb 1, 2021

Should we really need an automatical head response?

For those file routes, I think we can add HEAD methods specially.

Please send PRs if you think it resolved something, so we can discuss there.

@lunny lunny added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Feb 1, 2021
faridtsl added a commit to faridtsl/gitea that referenced this issue Feb 1, 2021
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.
6543 pushed a commit that referenced this issue Feb 5, 2021
* Add Content-Length header to HEAD requests

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 #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

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.

* Add Content-Length header to HEAD requests

Quick fix for imported dependency not used.

* Check if size is positiv int ...

Co-authored-by: zeripath <[email protected]>
@lunny lunny closed this as completed Feb 8, 2021
@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
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail
Projects
None yet
Development

No branches or pull requests

2 participants