talos session restore tests access hundreds of external resources

RESOLVED FIXED

Status

Testing
Talos
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jmaher, Assigned: Yoric)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

oh, over 33000 attempts, can this really be the case.  Today, the honorable :edmorley pushed a patch that printed a warning out for all external network access:
https://tbpl.mozilla.org/?tree=Try&rev=c088826545b9&jobname=talos

an example of a log with the errors is here:
https://tbpl.mozilla.org/php/getParsedLog.php?id=41959719&tree=Try&full=1

You can click on any of the 'o' tests and then view the brief or full log and your mouth will make a 'O'.  

What I see is access to:
abs.twimg.com
drive.google.com
ssl.gstatic.com

these are all part of the sessionrestore file which shouldn't be making network connections.  We either need to fix session restore to not connect automatically, or adjust the test to use local resources.
Yoric, can you comment on what the appropriate path here would be?
Flags: needinfo?(dteller)

Updated

4 years ago
Blocks: 1026970
Oh, I thought that this was benign. I'll prepare a fix.
Flags: needinfo?(dteller)
Created attachment 8442298 [details] [diff] [review]
Getting rid of these calls to the network

This patch replaces basically every "http://" and "https://" string in sessionstore.js with "http://localhost:8080/?", which should get rid of network accesses.
Assignee: nobody → dteller
Attachment #8442298 - Flags: review?(ttaubert)
Comment on attachment 8442298 [details] [diff] [review]
Getting rid of these calls to the network

Review of attachment 8442298 [details] [diff] [review]:
-----------------------------------------------------------------

Beautiful.
Attachment #8442298 - Flags: review?(ttaubert) → review+
Keywords: checkin-needed
landed:
https://hg.mozilla.org/build/talos/rev/02f95c7e7c06

once I get some preferences in talos and a few other fixes to network access done we will deploy!
Keywords: checkin-needed

Comment 6

4 years ago
Thank you everyone for filing, fixing and reviewing so quickly!
I thought I was going to have to go through and find/file/hassle deps of bug 1026970 myself hehe :-)
Status: NEW → ASSIGNED
Depends on: 1028999
Comment hidden (obsolete)
Comment hidden (obsolete)

Updated

4 years ago
Blocks: 617414
You need to log in before you can comment on or make changes to this bug.