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

Support HEAD requests #5153

Closed
2 of 7 tasks
lapin-b opened this issue Oct 23, 2018 · 5 comments · Fixed by #5186
Closed
2 of 7 tasks

Support HEAD requests #5153

lapin-b opened this issue Oct 23, 2018 · 5 comments · Fixed by #5186
Labels
type/enhancement An improvement of existing functionality
Milestone

Comments

@lapin-b
Copy link

lapin-b commented Oct 23, 2018

  • Gitea version (or commit ref): 1.5.2
  • Git version: 2.11.10
  • Operating system: Debian 9.5 (stretch)
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:
    Not relevant

Description

I regularly toot links to my repositories on Mastodon. Of course, the Federation effect kicks in and tons of HEAD requests arrive on my front server.

While I was watching them fail with a 404, I was thinking about HEAD requests support. "Why ?" you might ask. Because one could want to have a card displayed below their toot with a preview image, the repository name and description.

Basically, Mastodon first fires a HEAD requests to make sure the link leads to somewhere. Then, when the HEAD request has been successfully executed, it fires a GET request to get the contents of the page and generate an embed for the said link as shown below:

Mastodon embed card after successfully fetching a webpage contents

As we can see in the cURL output below, a HEAD request fails even though the repository really exists.

$ curl -I https://try.gitea.io/test_user1234/test-repository
HTTP/2 404 
content-type: text/html; charset=UTF-8
date: Tue, 23 Oct 2018 20:19:57 GMT
set-cookie: lang=en-US; Path=/; Max-Age=2147483647
set-cookie: i_like_gitea=[REDACTED]; Path=/; HttpOnly
set-cookie: _csrf=[REDACTED]; Path=/; Expires=Wed, 24 Oct 2018 20:19:57 GMT; HttpOnly
x-frame-options: SAMEORIGIN

PS: English is not my native language. Some sentences can look weird or not as crystal clear as I would like.

Screenshots

Not relevant

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Oct 24, 2018
@zeripath
Copy link
Contributor

Macaron provides a very simple way to add HEAD methods to every GET method:

m.SetAutoHead(true)

Which should just provide HEAD support for every GET on that router. However, each GET method would have to be checked to ensure it's idempotent. Another option would be to add HEAD methods to select methods that are require it.

@sapk
Copy link
Member

sapk commented Oct 24, 2018

GET request should always be idempotent (if not it should be fix) so using m.SetAutoHead(true) should be the good solution.

@zeripath
Copy link
Contributor

@sapk for HEAD it should probably be completely idempotent - GET doesn't always have to be completely idempotent due to tracking. If you're sure that the GETs are idempotent or idempotent enough then autohead should just solve this.

@sapk
Copy link
Member

sapk commented Oct 25, 2018

@zeripath we shouldn't have action made when you click on a link or load ressources (that will generate a GET request) for security reason. If not we should fix it so I am ok for autohead.

@zeripath
Copy link
Contributor

@sapk The below patch would be the correct place to call SetAutoHead.

diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index 4ca421065..5e54934a3 100644
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -134,6 +134,7 @@ func NewMacaron() *macaron.Macaron {
                DisableDebug: !setting.EnablePprof,
        }))
        m.Use(context.Contexter())
+       m.SetAutoHead(true)
        return m
 }

I guess the question is what tests do we need to do to check that this is a safe option.

zeripath added a commit to zeripath/gitea that referenced this issue Oct 29, 2018
techknowlogick pushed a commit that referenced this issue Oct 30, 2018
@lafriks lafriks added this to the 1.7.0 milestone Oct 30, 2018
@lafriks lafriks added type/enhancement An improvement of existing functionality and removed type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Oct 30, 2018
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants