Page MenuHomePhabricator

Explicitly limit lint and unit test names to 255 characters
Closed, ResolvedPublic

Description

See PHI61.

Probable reproduction steps:

  • Hack a linter to produce a message with a name longer than 255 characters, and make a lint error which it will catch.
  • Hack a unit test to produce a message with a name longer than 255 characters, and make a test which will fail.
  • Run arc diff.

Presumed actual behavior:

  • Unclear error about one or both name values being too long for the columns.

Underlying issue:

  • The HarbormasterBuildUnitMessage and HarbormasterBuildLintMessage classes have name fields defined as text255. Trying to report lint and unit results with longer names into Phabricator via harbormaster.sendmessage fails because the data is too long for the fields.

To fix:

  • Follow in the footsteps of D14165 and D14166 and explicitly limit these values to 255 characters in Arcanist, throwing an exception if they are too long.
  • Add similar explicit validation to the newFromDictionary(...) methods to do server-side validation (sharing the validation code seems pretty tricky).

Alternatively, we could change these to text fields to remove the limit, but we have other similar fields (code, severity) which have artificial-but-reasonable limits already, and explicit limits are consistent with that. It also doesn't seem like lint or unit messages with 255+ character names have much practical use. Lint messages also already have a separate (arbitrarily long) "description" field.