619567 - Passphrase/sync key entry should allow leading and trailing whitespace

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

Easy to do when copying and pasting from a text file.

Putting in Backend rather than UI, because this comes down to normalizePassphrase.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Strip leading and trailing whitespace. v1

Tested by clearing CollectionKeys and stomping on SyncKeyBundle, going through the post-error Update dialog and adding whitespace.

Tests pass, too.

Method for the above (in privileged Error Console):

// First: copy sync key!
Components.utils.import("resource://services-sync/service.js").Weave.Service.syncKeyBundle.keyStr = "a-abcde-abcde-abcde-abcde-abcde";
Components.utils.import("resource://services-sync/base_records/crypto.js").CollectionKeys.clear();
// Kick off sync

Comment 2

[Philipp von Weitershausen [:philikon]]

Comment on attachment 497962 [details] [diff] [review]
Strip leading and trailing whitespace. v1

Taking over from mconnor who's out.

Probably good idea to put on the 1.6.x branch as well. Thanks!

Comment 3

[Richard Newman [:rnewman]]

Pushed:

1.6.x: http://hg.mozilla.org/services/fx-sync/rev/6d5db9467f9b
default: http://hg.mozilla.org/services/fx-sync/rev/b4d631a9b144

Comment 4

[Tracy Walker [:tracy]]

tested this against Minefield nightly of 2011-01-04.  pasted correct sync key in dialog for adding device, not using easy setup, added a space to the beginning and/or end of the key.  Also tested ensuring there was whitespace present in CnP

I get Wrong key error message in the UI. However, if I removed the spaces, setup accepts the key as expected.

Comment 5

[Richard Newman [:rnewman]]

Well, that's weird. Will take a look later.

Comment 6

[Tracy Walker [:tracy]]

Hmmm, it just worked on 1.6.1

Comment 7

[Richard Newman [:rnewman]]

... it's in m-c:

http://hg.mozilla.org/mozilla-central/rev/dbed066cf3fb

which suggests that perhaps the UI isn't calling normalizePassphrase, or a subsequent commit screwed things up.

Comment 8

[Richard Newman [:rnewman]]

(In reply to comment #6)
> Hmmm, it just worked on 1.6.1

=> the built-in Fx4 UI is misbehaving. That's a good lead.

Comment 9

[Philipp von Weitershausen [:philikon]]

(In reply to comment #7)
> which suggests that perhaps the UI isn't calling normalizePassphrase, or a
> subsequent commit screwed things up.

The built-in UI sure calls normalizePassphrase.

Comment 10

[Tracy Walker [:tracy]]

so this doesn't work on m-c nightly of 2010-12-21.  So, unless another check-in between when the fix here was pushed (see comment #3) and when the tested nightly was created broke this, this was never working on trunk.

Comment 11

[Tracy Walker [:tracy]]

Does fact that 1.6.1 has a user/pwd dialog, then a sync key dialog, but trunk has all three fields combined in one dialog have anything to do with this?

Comment 12

[Richard Newman [:rnewman]]

I just failed to reproduce this with a 2011-01-05 Minefield build from this morning.

Used a profile that I know is syncing with another profile. Clicked "Stop using this account", made sure no Sync add on was installed, restarted Minefield.

Set up > Don't have device > entered credentials. Put three spaces at start and end of key. No error.

Verified that Sync completed without error. Nothing in error console.

Looking in Password Manager shows my entered 26-char sync key with no hyphens or whitespace.

Tracy, would you please test with tomorrow morning's nightly, and re-close if you can't repro?

Comment 13

[Tracy Walker [:tracy]]

Hmmm, I was using a crufty profile when I reopened this.  Try with the MF build of 2011-01-06 with a fresh profile, this works.

Comment 14

[Tracy Walker [:tracy]]

verified on both 1.6.1 and trunk