Skip to content

Limit the maximum length of release description [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Shinya Maeda requested to merge limit-release-description-field into master

NOTE: This is an urgent MR in the context.

What does this MR do?

As we discussed in the thread, Release List API is quite slow due to rendering HTML description for each release entry. 80-90% of the response time is spent on this process. Especially, the timing could be extremely longer when the description is very long, like length > 1 megabyte.

We're currently lacking the limitation on the length of the description field. This should be limited properly according to the GitLab application limits guideline. This MR adds AR validation for the field. See #299447 (comment 561339710) for more info.

Related #299447 (closed)

Screenshots (strongly suggested)

Prepare a request with a long description

# Create 1 megabyte + 1 byte description
tr -dc A-Za-z0-9 </dev/urandom | head -c 1000001 > large.txt

# Create a request file
echo "{ \"name\": \"New release\", \"tag_name\": \"v0.3\", \"description\": \""$(cat large.txt)"\" }" > /home/shinya/workspace/thin-gdk/services/rails/src/example.json

Execute the request and receive the expected error

shinya@shinya-B550-VISION-D:~/workspace/thin-gdk/services/rails/src$ curl -i \
>     --header 'Content-Type: application/json' \
>     --header "PRIVATE-TOKEN: ydBM1d29P5NzWynqYPSE" \
>     --data-binary "@/home/shinya/workspace/thin-gdk/services/rails/src/example.json" \
>     --request POST "http://local.gitlab.test:8181//api/v4/projects/34/releases"
HTTP/1.1 100 Continue

HTTP/1.1 400 Bad Request
Cache-Control: no-cache
Content-Type: application/json
Vary: Origin
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Gitlab-Feature-Category: release_orchestration
X-Request-Id: 01F4BQD888NEXHQ0AWJ5YM37DT
X-Runtime: 0.132807
Date: Wed, 28 Apr 2021 07:52:01 GMT
Content-Length: 88

{"message":"Validation failed: Description is too long (maximum is 1000000 characters)"}

How to check the maximum lengths in the current rows in production

SELECT project_id, length(description)
FROM releases
WHERE description IS NOT NULL
ORDER BY length(description) DESC
LIMIT 10;

Currently, there are no records on production that exceed the limitation (technically, there is a few but they are test instance). So the risk of this change is nearly zero.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Shinya Maeda

Merge request reports