Page MenuHomePhabricator
Create Task
Maniphest T11567

Allow dashes in monospace font preferences
Closed, ResolvedPublic
Actions

Description

Currently the monospaced font setting in user preferences doesn't allow dashes. This prevents valid values (such as Inconsolata-dz) and the CSS generic font-family sans-serif (admittedly a bizarre choice for monospaced font, but some prefer proportional even when the monospaced makes sense).

Reproduction:

  • Go to Personal or Global Display Preferences
  • Enter 13px sans-serif as the Monospaced Font
  • Click Save Changes

Anticipated results:

  • Monospaced font is changed to sans-serif

Actual results:

  • Error stating Monospaced font value "13px sans-serif" is unsafe. You may only enter letters, numbers, spaces, commas, periods, forward slashes and double quotes.

Version:
phabricator 2201c65eb73fb99b8625bea45c273d262f2c289f (Tue, Aug 23)
arcanist 89e8b48523844cc3eff8b775f8fae49e85f8fc22 (Fri, Aug 19)
phutil 237549280f08feb8852857b1d109dfb0aba345f0 (Mon, Aug 22)

Revisions and Commits

rP Phabricator
D16519rPf712ae718ccc Added `-` to the whitelist for CSS rules

Related Objects

Event Timeline

• jgdye created this task.Aug 31 2016, 12:30 PM
epriestley added a subscriber: epriestley.Aug 31 2016, 12:35 PM

I believe this is fine; I can't immediately come up with an attack using -.

chad added a project: User Preferences.Aug 31 2016, 4:54 PM
avivey renamed this task from Allow dashes in monospace font to Allow dashes in monospace font preferences.Aug 31 2016, 5:52 PM
avivey updated the task description. (Show Details)
epriestley assigned this task to jcox.Sep 1 2016, 2:05 PM
epriestley edited projects, added Contributor Onboarding; removed Bug Report.
  • Try to come up with a way that - can do something dangerous when injected into a CSS rule.
    • See D7274#47131 for a template of how to attack this.
    • If you can come up with any attacks, don't implement this.
  • Adjust the regular expression in PhabricatorMonospacedFontSetting::filterMonospacedCSSRule() to include -.
  • Adjust the error text in PhabricatorMonospacedFontSetting->validateTransactionValue() to say "..., hyphens, ..." or similar.
Herald added a subscriber: tycho.tatitscheff. · View Herald TranscriptSep 1 2016, 2:05 PM
epriestley triaged this task as Low priority.Sep 1 2016, 2:06 PM
epriestley moved this task from Backlog to Basic on the Contributor Onboarding board.
jcox added a revision: D16519: Added `-` to the whitelist for CSS rules.Sep 8 2016, 8:25 PM
jcox closed this task as Resolved by committing rPf712ae718ccc: Added `-` to the whitelist for CSS rules.Sep 8 2016, 8:29 PM
jcox added a commit: rPf712ae718ccc: Added `-` to the whitelist for CSS rules.
Phabricator · Built by Phacility