Closed Bug 1600897 Opened 4 years ago Closed 4 years ago

User profile name replaced with {???} with unichar and long charaters on rename

Categories

(Toolkit :: Startup and Profile System, defect, P5)

defect

Tracking

()

VERIFIED FIXED
mozilla73
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- verified
firefox74 --- verified

People

(Reporter: cfogel, Assigned: stas)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached file new 1.txt

Affected versions

  • 73.0a1 (2019-12-02), 72.0b1, 71.0b12, 71.0;

Affected platforms

  • Windows 10, macOS 10.15;

Steps to reproduce

  1. Launch Firefox with a valid profile;
  2. Access about:profiles
  3. Click on any profile that is not in use and click on the Rename button
  4. Input the string from the attached text document
    ** (repeat инсд@ēș#☈☎☢𐌸置七dcdcdssfsdefrecrtvweth wethwljtwekwekjtlerwl many times)
  5. Click on the Ok button;

Expected result

  • name is truncated;

Actual result

  • name of string is replaced with {???}

Regression range

  • Last good build date: 2019-07-29
  • First bad build date: 2019-07-30
  • Pushlog URL

Additional notes

  • attached with the document is the string for the character rename;
  • trying to create the profile from both the profile manager and the about:profiles page with that name give an (expected) fail message.

Edge case that was already affecting 70, does not look like a good fit for an uplift to release, wontfix 71.

Has Regression Range: --- → yes
Has STR: --- → yes

(In reply to Cristian Fogel, QA [:cfogel] from comment #0)

Regression range

  • Last good build date: 2019-07-29
  • First bad build date: 2019-07-30
  • Pushlog URL

This pushlog link doesn't match the dates you provided - can you provide the correct pushlog please?

Flags: needinfo?(cristian.fogel)

Dates are valid, pushlog link was not. Thanks for spotting this!

  • Pushlog URL
  • mozregressions points to bug 1568914 as potential regressor.
Flags: needinfo?(cristian.fogel)

This is because of Fluent's limits on the length of variables (2.5k string length). We also saw this in bug 1539000.

Mossop, is there a practical limit on the size of profile names? Should we just limit it to 500 or 1000 characters or something, which should be enough for everybody?

Also, Zibi, does this type of limit exist for .properties ? It feels a bit arbitrary, and it'll keep biting us in different places where a user-controlled value is substituted into a string like this.

Flags: needinfo?(gandalf)
Flags: needinfo?(dtownsend)
See Also: → 1539000

(FWIW, I don't actually think this bug in itself is super important given the usecase. But the generic issue is probably something we should keep an eye on.)

Regressed by: 1568914

Apparently, older Fluent silently truncated overlong strings.
https://hg.mozilla.org/mozilla-central/rev/1b513d98a80e#l26.546
Fluent 0.14.0 would throw an error instead. So initial value ("???") would be used.
https://hg.mozilla.org/mozilla-central/rev/1b513d98a80e#l26.79

I don't think a similar limit exists for .properties, because strings in .properties can't reference other strings. The limit has been put in place in Fluent to protect against deeply nested message/term references which could take too long to resolve. The way it's currently implemented treats variable references in the same manner as messages/term references, which is why we're seeing this re-surface. Perhaps there's no need to enforce the length limit on variables?

(In reply to :Gijs (he/him) from comment #4)

Mossop, is there a practical limit on the size of profile names? Should we just limit it to 500 or 1000 characters or something, which should be enough for everybody?

Currently we don't apply any limit and off the top of my head I can't think of anything else that would be causing a limit right now. I can't imagine limiting to 500 characters would be an issue (he says knowing full well there is probably someone out there using more than that).

Flags: needinfo?(dtownsend)
Flags: needinfo?(gandalf)

(In reply to Staś Małolepszy :stas from comment #7)

Perhaps there's no need to enforce the length limit on variables?

Zibi, is this something we could consider doing?

Flags: needinfo?(gandalf)

Zibi, is this something we could consider doing?

Yeah. I'm not opposed to that, but that's Fluent design decision, so in the hands of Stas :)

Flags: needinfo?(gandalf)
See Also: → 1602640

There's at least one planned feature in Fluent 1.x which will blur the line between message reference and variable references. I was thinking out loud above, but for now, I'd prefer to not change Fluent code for this bug, if possible.

Doesn't sound like we'll fix this for 72.

The priority flag is not set for this bug.
:mossop, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(dtownsend)
Flags: needinfo?(dtownsend)
Priority: -- → P5

@fluent/bundle 0.14.1 removed the arbitrary limitation on the length of variables. I vendored the update in bug 1604839.

Cristian, can you please update on the status of this bug after the landing of bug 1604839? Thanks!

Flags: needinfo?(cristian.fogel)

Indeed, it appears as fixed for 74.0a1 (2020-01-13), 73.04.
72.0.1 is still affected.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(cristian.fogel)
Resolution: --- → FIXED
Attached image Screenshot_2.png

Another thing worth mentioning, is that the current situation popped up.

  • tried renaming the long_string profile from the about:profiles page.

Should we file a new bug for it, at least to bring the Windows settings to mac/Ubuntu behaviour?
Couldn't find anything logged in this area.

Sure, please file a new bug for that. Thanks for checking!

Assignee: nobody → stas
Status: RESOLVED → VERIFIED
Depends on: 1604839
Target Milestone: --- → mozilla73
See Also: → 1609159

With pleasure, marked it as bug 1609159.

You need to log in before you can comment on or make changes to this bug.