Page MenuHomePhabricator

Convert Phabricator to handle "%s" / "%B" properly
ClosedPublic

Authored by epriestley on Feb 23 2014, 9:54 PM.

Details

Summary

Ref T1191. I believe we only have three meaningful binary fields across all applications:

  • The general cache may contain gzipped content.
  • The file storage blob may contain arbitrary binary content.
  • The Passphrase secret can store arbitrary binary data (although it currently never does).

This adds Lisk config for binary fields, and uses %B where necessary.

Test Plan
  • Added and executed unit tests.
  • Forced file uploads to use MySQL, uploaded binaries.
  • Disabled the CONFIG_BINARY on the file storage blob and tried again, got an appropraite failure.
  • Tried to register with an account containing a G-Clef, and was stopped before the insert.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

src/aphront/console/DarkConsoleCore.php
77โ€“84

This isn't precisely related, but was triggered during testing.

src/applications/cache/PhabricatorKeyValueDatabaseCache.php
22

This data may be compressed.

src/applications/files/storage/PhabricatorFileStorageBlob.php
17โ€“44

This never worked, see discussion in D3392.

epriestley updated this revision to Unknown Object (????).Feb 23 2014, 10:16 PM
  • The PushLog and RefCursor tables also store ref names in the raw. Mark those fields as binary.

Accepting as this code looks good, but I haven't done any type of exhaustive search for columns that need to be switched to binary.

I identified binary columns using these strategies:

  • Remembering where we'd used binary columns using my mind.
  • Grepping quickstart.sql (which is a comprehensive, recent dump of all schema) for BINARY, LONGBLOB, and latin1_bin.

It's possible something escaped me, but even if I did miss something the consequences are probably not severe, and the resulting error should be easy to identify and resolve. (For example, the refNameRaw columns store binary data only if you've managed to create a non-utf8 branch name in Git or Mercurial, which is probably possible but very unlikely.)