Opened 4 years ago

Closed 4 years ago

#19348 closed defect (fixed)

Windows %BUILD_TARGET% changed in Tor Browser 6.0

Reported by: boklm Owned by: boklm
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-6.0-issues, TorBrowserTeam201606R
Cc: tbb-team, mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In previous Tor Browser releases, the %BUILD_TARGET% used in the updater URL in Windows versions was WINNT_x86-gcc3. In Tor Browser 6.0, this changed to WINNT_x86-gcc3-x64, which broke our updater URLs.

A quick fix was to apply this patch, and regenerate the .htacess we use to deliver the update manifests:

diff --git a/tools/update-responses/config.yml b/tools/update-responses/config.yml
index 5e4d463f9262..4595544ec21e 100644
--- a/tools/update-responses/config.yml
+++ b/tools/update-responses/config.yml
@@ -5,7 +5,7 @@ download:
 build_targets:
     linux32: Linux_x86-gcc3
     linux64: Linux_x86_64-gcc3
-    win32: WINNT_x86-gcc3
+    win32: WINNT_x86-gcc3(-x64)?
     osx32: Darwin_x86-gcc3
     osx64: Darwin_x86_64-gcc3
 channels:

However this change breaks the check_update_responses_deployement command, which uses the build_targets values in test URLs (without expecting them to be regexps), so other changes are required to fix that part.

Child Tickets

Attachments (2)

0001-Bug-19348-Windows-BUILD_TARGET-changed-in-Tor-Browse.patch (4.0 KB) - added by boklm 4 years ago.
htaccess-alpha (14.8 KB) - added by boklm 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by gk

Keywords: tbb-6.0-issues added

comment:2 Changed 4 years ago by mcs

Kathy and I should have noticed this when we tested the 6.x updater, but the .htaccess file on our test server uses patterns like ^WINNT_x86-.*/[^/]+/en-US, so we just got lucky that our tests worked (and unlucky that we did not notice the -x64 part).

After some detective work we found the culprit:

https://hg.mozilla.org/mozilla-central/rev/2a1163cd7736

Mozilla added the -64 to allow them to update people from 32-bit Firefox to 64-bit Firefox (although as far as I know they have not done that yet).

I think someone with an old CPU might generate requests that include WINNT_x86-gcc3-x86, so we should allow for that too.

Do we already leak the processor architecture? If the answer is "yes" we may want to just keep this as-is. If not, we should patch the ABI getter inside toolkit/modules/UpdateUtils.jsm (where the code now lives) to omit the gWinCPUArch part.

comment:3 in reply to:  2 Changed 4 years ago by boklm

Replying to mcs:

I think someone with an old CPU might generate requests that include WINNT_x86-gcc3-x86, so we should allow for that too.

I have now updated again the .htaccess in the release and alpha channels after generating it with this, to allow both -x64 and -86:

diff --git a/tools/update-responses/config.yml b/tools/update-responses/config.yml
index 5e4d463f9262..1d8637a572fd 100644
--- a/tools/update-responses/config.yml
+++ b/tools/update-responses/config.yml
@@ -5,7 +5,7 @@ download:
 build_targets:
     linux32: Linux_x86-gcc3
     linux64: Linux_x86_64-gcc3
-    win32: WINNT_x86-gcc3
+    win32: WINNT_x86-gcc3(-x64|-x86)?
     osx32: Darwin_x86-gcc3
     osx64: Darwin_x86_64-gcc3
 channels:
Last edited 4 years ago by boklm (previous) (diff)

comment:4 Changed 4 years ago by bugzilla

Mozilla added the -64 to allow them to update people from 32-bit Firefox to 64-bit Firefox (although as far as I know they have not done that yet).

Because they are so feature-different as separate products ;)

I think someone with an old CPU might generate requests that include WINNT_x86-gcc3-x86, so we should allow for that too.

All our machines with 32-bit Windows of all versions had been updated through WINNT_x86-gcc3-x86 before this ticket was created :) And no, most of them have x64 not old CPUs ;)

Do we already leak the processor architecture?

So this is only the Edition of Windows (x86 or x64), despite of how Mozilla calls it.
But according to #19316, they are going to detect and report far more info than just CPU arch...

Changed 4 years ago by boklm

Attachment: htaccess-alpha added

comment:5 Changed 4 years ago by boklm

Keywords: TorBrowserTeam201606R added
Status: newneeds_review

I attached a patch to fix that, allowing multiple values to be specified in the build_targets. I also attached the .htaccess file generated for the alpha channel using this patch.

comment:6 in reply to:  5 Changed 4 years ago by mcs

Replying to boklm:

I attached a patch to fix that, allowing multiple values to be specified in the build_targets. I also attached the .htaccess file generated for the alpha channel using this patch.

r=brade, r=mcs
These changes look good to us. There are a couple of typos in the checkin comment that should be fixed:

s/.htacess/.htaccess/
s/Aditionally/Additionally/

comment:7 Changed 4 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

I fixed the typos and applied the patch to all branches (master, maint-6.0, hardened-builds, hardened-builds-6.5: 7124bb8dfb187108f0f2a4d885ef8f32afea1af7, 417c30014f40ebb40d7b03f0a71609e0122846e7, 8944fbfc435fbba739fdbae74402409b0b4f58f7 and 4569eee1f94ad008c9a4b3d3924e6fed0b2e1926.

Note: See TracTickets for help on using tickets.