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

gitea webhook empty POST variable #7700

Closed
2 of 7 tasks
8ctopus opened this issue Aug 1, 2019 · 6 comments
Closed
2 of 7 tasks

gitea webhook empty POST variable #7700

8ctopus opened this issue Aug 1, 2019 · 6 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug

Comments

@8ctopus
Copy link
Contributor

8ctopus commented Aug 1, 2019

Description

Upgrading from gitea 1.8.3 to 1.9.0 breaks gitea webhooks of type application/x-www-form-urlencoded: the POST variable is empty and therefore doesn't contain the payload.

sample hook on receiver side:
<?php
file_put_contents('/var/tmp/gitea-debug.log', var_export($_POST, true));
exit();

result in 1.9.0:
array ( )

result in 1.8.3:
array ( 'payload' => '{ "secret": "***********", "ref": "refs/heads/master", "before": "3bc52c056257af7ae79fe5399f59f5de506b877c", "after": "3bc52c056257af7ae79fe5399f59f5de506b877c", "compare_url": "", "commits": [ ... }', )

UPDATE: It looks like the same issue as this one #7692

Screenshots

2019-08-01_152635

@stale
Copy link

stale bot commented Oct 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Oct 1, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Oct 1, 2019
@stale stale bot removed the issue/stale label Oct 1, 2019
@8ctopus
Copy link
Contributor Author

8ctopus commented Oct 1, 2019

Problem is still present in version 1.9.3...
switching the webhook POST content type from application/x-www-form-urlencoded to application/json fixes the problem if the recipient php code is updated accordingly.

...

$content_type = isset($_SERVER['CONTENT_TYPE']) ? strtolower(trim($_SERVER['CONTENT_TYPE'])) : '';

switch ($content_type)
{
    case 'application/json':
        // get RAW post data
        $payload = trim(file_get_contents("php://input"));
        break;

    case '':
        // get payload
        $payload = @$_POST['payload'];
        break;

    default:
        error_log($log_base .' - FAILED - unknown content type - '. $content_type);
        header('HTTP/1.0 401 Unauthorized');
        exit();
}

// check payload exists
if (empty($payload))
{
    error_log($log_base .' - FAILED - no payload');
    header('HTTP/1.0 401 Unauthorized');
    exit();
}

// convert json to array
$decoded = json_decode($payload, true);

...

@8ctopus
Copy link
Contributor Author

8ctopus commented Oct 2, 2019

looks like the payload is no longer passed correctly.

models/webhook.go deliver() function code in 1.8.3

func (t *HookTask) deliver() {
	...
	req := httplib.Post(t.URL).SetTimeout(timeout, timeout).
		Header("X-Gitea-Delivery", t.UUID).
		Header("X-Gitea-Event", string(t.EventType)).
		Header("X-Gogs-Delivery", t.UUID).
		Header("X-Gogs-Event", string(t.EventType)).
		HeaderWithSensitiveCase("X-GitHub-Delivery", t.UUID).
		HeaderWithSensitiveCase("X-GitHub-Event", string(t.EventType)).
		SetTLSClientConfig(&tls.Config{InsecureSkipVerify: setting.Webhook.SkipTLSVerify})

	switch t.ContentType {
		case ContentTypeJSON:
			req = req.Header("Content-Type", "application/json").Body(t.PayloadContent)
		case ContentTypeForm:
			req.Param("payload", t.PayloadContent)
	}

and code in HEAD revision on master

func (t *HookTask) deliver() error {
	t.IsDelivered = true

	var req *http.Request
	var err error

	switch t.HTTPMethod {
	case "":
		log.Info("HTTP Method for webhook %d empty, setting to POST as default", t.ID)
		fallthrough
	case http.MethodPost:
		switch t.ContentType {
		case ContentTypeJSON:
			req, err = http.NewRequest("POST", t.URL, strings.NewReader(t.PayloadContent))
			if err != nil {
				return err
			}

			req.Header.Set("Content-Type", "application/json")

		case ContentTypeForm:
			var forms = url.Values{
				"payload": []string{t.PayloadContent},
			}

			req, err = http.NewRequest("POST", t.URL, strings.NewReader(forms.Encode()))
			if err != nil {

				return err
			}
		}

@vszakats
Copy link
Contributor

vszakats commented Oct 20, 2019

As of v1.9.4, the issue is still there. It seems that the Content-Type: application/x-www-form-urlencoded header is missing from the POST request.

May be related to changing the http client from GiteaServer to Go-http-client/1.1 between 1.8.3 and 1.9.x. Maybe the former set this by default, but the new client needs to have it set explicitly [UPDATE: Yes, confirmed, see PR]:

POST / HTTP/1.1
Host: localhost:9090
User-Agent: Go-http-client/1.1
Content-Length: 4224
X-GitHub-Delivery: ffffffff-0000-0000-0000-ffffffffffff
X-GitHub-Event: push
X-Gitea-Delivery: ffffffff-0000-0000-0000-ffffffffffff
X-Gitea-Event: push
X-Gitea-Signature: 
X-Gogs-Delivery: ffffffff-0000-0000-0000-ffffffffffff
X-Gogs-Event: push
X-Gogs-Signature: 
Accept-Encoding: gzip

payload=[...]

Compared to application/json mode, where Content-Type is there:

POST / HTTP/1.1
Host: localhost:9090
User-Agent: Go-http-client/1.1
Content-Length: 2916
Content-Type: application/json
X-GitHub-Delivery: ffffffff-0000-0000-0000-ffffffffffff
X-GitHub-Event: push
X-Gitea-Delivery: ffffffff-0000-0000-0000-ffffffffffff
X-Gitea-Event: push
X-Gitea-Signature: 
X-Gogs-Delivery: ffffffff-0000-0000-0000-ffffffffffff
X-Gogs-Event: push
X-Gogs-Signature: 
Accept-Encoding: gzip

{
  "secret": [...]

@vszakats
Copy link
Contributor

It looks like this is what happened. Issued a PR to fix it.

techknowlogick pushed a commit that referenced this issue Oct 20, 2019
This header is missing since switching http client from GiteaServer (`code.gitea.io/gitea/modules/httplib`) to Go-http-client/1.1 (`net.http`). The header [was added by default](https://github.com/go-gitea/gitea/blob/release/v1.8/modules/httplib/httplib.go#L301) by the former, but this is no longer true with `net.http`, so it needs to be done explicitly:

Ref: #7700
@adelowo
Copy link
Member

adelowo commented Oct 20, 2019

Can we close this issue?

zeripath pushed a commit that referenced this issue Oct 20, 2019
This header is missing since switching http client from GiteaServer (`code.gitea.io/gitea/modules/httplib`) to Go-http-client/1.1 (`net.http`). The header [was added by default](https://github.com/go-gitea/gitea/blob/release/v1.8/modules/httplib/httplib.go#L301) by the former, but this is no longer true with `net.http`, so it needs to be done explicitly.

Closes: #7700
@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
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
Development

No branches or pull requests

6 participants