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

Too large webhook's response (>65K response) causes the webhook to be sent indefinitely #3561

Closed
2 of 7 tasks
juanpablo-santos opened this issue Feb 22, 2018 · 6 comments
Closed
2 of 7 tasks
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality

Comments

@juanpablo-santos
Copy link
Contributor

  • Gitea version (or commit ref): 1.3.2
  • Git version: 2.14
  • Operating system: Gitea Docker image
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant (don't know if it is using mysql)
  • Log gist:

Description

For background see jenkinsci/generic-webhook-trigger-plugin#42, but basically when a webhook's response is too big to not fit into hook_task.response_content (>65K response for MySQL), the webhook is fired, but it doesn't get updated (is_released, is_succeed and released remain at 0. Haven't tried to see what happens when there is too much information too on the request_content or the payload_content (e.g. a push webhook with way too many commits)

@lunny
Copy link
Member

lunny commented Feb 22, 2018

hook_task.response_content is designed as TEXT in mysql, which's max size is 65,535 bytes about 64KB. So maybe we should use MEDIUMTEXT or LONGTEXT for this column.

@juanpablo-santos
Copy link
Contributor Author

Hi @lunny,

initially I thought of changing the column too, but according to https://stackoverflow.com/a/6766854, MEDIUMTEXT allows up to 16Mbits and LONGTEXT up to 64Gbits, which seemed too overkill to me.

As the response appears to be used only on the webhook response panel, which by default only shows the last ten webhooks calls, and given that usually the system pinged will have its own logs too, wouldn't it make more sense to trim the response to, something like 60K, f.ex, and then append some text stating that the response has been trimmed? In anycase, we're not doing anything special with that table/columns, so going to MEDIUMTEXT would be ok for us.

also, what would happen now when the payload is too big to fit into hook_task.payload_content?

thanks,
juan pablo

@lunny lunny added the type/enhancement An improvement of existing functionality label May 18, 2018
@stale
Copy link

stale bot commented Jan 26, 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 Jan 26, 2019
@juanpablo-santos
Copy link
Contributor Author

Hi,

in order to avoid the stale bot, anything else needed on this issue?

thx,
juan pablo

@techknowlogick techknowlogick added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented and removed issue/stale labels Jan 27, 2019
@kalvaro1215
Copy link

kalvaro1215 commented Apr 7, 2021

Hi,

I'm facing the same issue here with 1.13.0 :-)
The issue appears with payload_content field and doesn't seems to fire up the webhook
As mentionned, updating the SQL type to MEDIUMTEXT seems to be the correct approach but changing it may break futur Gitea's migration script...

BR,

zeripath pushed a commit that referenced this issue Jun 19, 2022
Mysql TEXT has a limit of 64KB, change this to LONGTEXT in mysql only so we can have bigger hook payloads.

Postgresql has unlimited TEXT - https://www.postgresql.org/docs/current/datatype-character.html
Sqlite has unlimited TEXT - https://www.sqlitetutorial.net/sqlite-data-types/#:~:text=The%20maximum%20length%20of%20TEXT,SQLite%20supports%20various%20character%20encodings.

Same issue as #16656 but for hook_task

Fixes #10252, #19679, #3561
@42wim
Copy link
Member

42wim commented Jun 19, 2022

Fixed in #20038

@42wim 42wim closed this as completed Jun 19, 2022
vsysoev pushed a commit to IntegraSDL/gitea that referenced this issue Aug 10, 2022
Mysql TEXT has a limit of 64KB, change this to LONGTEXT in mysql only so we can have bigger hook payloads.

Postgresql has unlimited TEXT - https://www.postgresql.org/docs/current/datatype-character.html
Sqlite has unlimited TEXT - https://www.sqlitetutorial.net/sqlite-data-types/#:~:text=The%20maximum%20length%20of%20TEXT,SQLite%20supports%20various%20character%20encodings.

Same issue as go-gitea#16656 but for hook_task

Fixes go-gitea#10252, go-gitea#19679, go-gitea#3561
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this issue Aug 24, 2022
Mysql TEXT has a limit of 64KB, change this to LONGTEXT in mysql only so we can have bigger hook payloads.

Postgresql has unlimited TEXT - https://www.postgresql.org/docs/current/datatype-character.html
Sqlite has unlimited TEXT - https://www.sqlitetutorial.net/sqlite-data-types/#:~:text=The%20maximum%20length%20of%20TEXT,SQLite%20supports%20various%20character%20encodings.

Same issue as go-gitea#16656 but for hook_task

Fixes go-gitea#10252, go-gitea#19679, go-gitea#3561
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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/enhancement An improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

5 participants