Skip to content

Validate project path prior to hitting the database.

Andreas Brandl requested to merge ab-45247-project-lookups-validation into master

What does this MR do?

When we lookup a project on any of the API endpoints, there are two options:

  1. By ID (integer)
  2. By path (format: namespace/project-name)

For the latter, we don't validate the format and go straight to the database to find the Project given its full path. However, we have seen examples of this where we're given an illegal path (because of its format) and still try to find this in the database.

This MR improves the situation and validates the given path before hitting the database. If it is not valid, we return nil which will trigger a 404 response.

This can be tested with a call like:

curl -v http://localhost:3000/api/v4/projects/undefined\?private_token\=...

Without this changes, it triggers the following SELECT queries:

LOG:  statement: SELECT 1
LOG:  execute a37: SELECT  "personal_access_tokens".* FROM "personal_access_tokens" WHERE "personal_access_tokens"."token" = $1 LIMIT 1
LOG:  execute <unnamed>: SELECT "users".* FROM "users" WHERE "users"."id" IN (45)
LOG:  execute a38: SELECT  "projects".* FROM "projects" INNER JOIN "routes" ON "routes"."source_id" = "projects"."id" AND "routes"."source_type" = $1 WHERE ((LOWER(routes.path) = LOWER('undefined')))  ORDER BY (CASE WHEN  routes.path = 'undefined' THEN 0 ELSE 1 END) LIMIT 1

With the change, we omit the last query on projects table.

Why was this MR needed?

See https://gitlab.com/gitlab-org/gitlab-ce/issues/45247 and https://gitlab.com/gitlab-com/infrastructure/issues/4006.

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/45247

Edited by Yorick Peterse

Merge request reports