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

[WIP]Fix column type and validation for long webhook URL #1977

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

int128
Copy link
Contributor

@int128 int128 commented Apr 26, 2018

This fixes #1951.

Before submitting a pull-request to GitBucket I have first:

  • read the contribution guidelines
  • rebased my branch over master
  • verified that project is compiling
  • verified that tests are passing
  • squashed my commits as appropriate (keep several commits if it is relevant to understand the PR)
  • marked as closed using commit message all issue ID that this PR should correct

@int128
Copy link
Contributor Author

int128 commented Apr 26, 2018

I will check that MySQL, PostgreSQL and H2 supports column change to TEXT from VARCHAR. Please merge after that.

@int128
Copy link
Contributor Author

int128 commented Apr 26, 2018

Oops. It needs an index for TEXT column but it is not supported. Any ideas?
A webhook URL should be longer than 255 and cannot be VARCHAR.

@takezoe
Copy link
Member

takezoe commented Apr 26, 2018

I have two ideas:

  • Modify (or remove) constraints and keep consistency by application layer
  • Add alternative columns which store hash value of text columns, and use these columns for constraints

I think the second one is a right way but it needs complex migration.

@ms502040
Copy link
Contributor

ms502040 commented Nov 4, 2018

I exactly dont'n know about H2 and pgsql, but mysql max varchar is 65,535 and max row length 65,535 bytes too The CHAR and VARCHAR Types, Row Size Limits:
H2 Limits and Limitations

There is no limit for the following entities, except the memory and storage capacity: maximum identifier length (table name, column name, and so on); maximum number of tables, columns, indexes, triggers, and other database objects; maximum statement length, number of parameters per statement, tables per statement, expressions in order by, group by, having, and so on; maximum rows per query; maximum columns per table, columns per index, indexes per table, lob columns per table, and so on; maximum row length, index row length, select row length; maximum length of a varchar column, decimal column, literal in a statement.

pgsql Character Types:

In any case, the longest possible character string that can be stored is about 1 GB

There is problem with max key length (in mysql 1024bytes, i think), but why indexing URL column? maybe you have reason for unique url per repository? (except noob users :) ) maybe better only index USER_NAME + REPOSITORY_NAME for webhook search when run it? little index = fast search
edit
for foreign key constraints to WEB_HOOK_EVENT set id (autoincrement, bigint) and use join for select

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

AWS CodeBuild requires a very long webhook URL and token
4 participants