projectURL uses http and https inconsistently for phet.colorado.edu references

samreid commented 3 years ago

From https://github.com/phetsims/number-line-distance/issues/64 my IDE said http links are not secure:

I confirmed that our homepage is https, and converted to use https. Then we discussed over slack that most references use http, and only a fraction use https.

Chris Malley  10:01 AM
For the 2nd commit, https://github.com/phetsims/number-line-distance/commit/43a1a113088aea68e5c6a2a452e9dd75c86a8492 . I see 100+ occurrences of "projectURL": "http://phet.colorado.edu", in other license.json files. (edited) 
10:01
Should they be changed to https?
10:02
And does it matter in license.json?  http works just fine, and redirects to https. (edited) 

Sam Reid  10:05 AM
How many “https”?  I can’t count easily because those are all ignored by my IDE.

Jesse Greenberg  10:06 AM
Is there a way to see error logs for phettest? I am still only seeing adapted-from-phet brand for john-travoltage build on phettest

Chris Malley  10:06 AM
1356 http vs 83 https (edited) 

Sam Reid  10:07 AM
I’ll create an issue

Chris Malley  10:07 AM
Like I said, I think it’s a non-issue in license.json.
10:07
"projectURL": "phet.colorado.edu",
… would even be OK.

Sam Reid  10:08 AM
To be specific, you are recommending no issue and no further work on this aspect?

Chris Malley  10:08 AM
Don’t care.
New
10:09
They are all valid URLs that work in modern browsers, and get the reader to the PhET site. If you feel that it’s important that they all be identical, then feel free to create an issue.

Perhaps it would be easy for a developer to unify these so we don't run into this discrepancy again. Since our homepage is https://phet.colorado.edu, that seems the most appropriate to use.

I have mipmaps/images/etc directories ignored in my IDE, so perhaps another developer would have an easier time of doing this. I'll randomly assign to another developer, they can assign to someone else if they wish.

jbphet commented 3 years ago

I've fixed the references in all the license.json files, *.md files, and a number of .js files.

I unfortunately blew the commit message on many of the changes to license.json and didn't reference this issue. I think I was paying too much attention to not breaking anything and spaced updating the commit message and ended up using an old message. Regardless, they should now be correct.

Two follow-up notes for @samreid:

I was unable to update the kite repo because some automated tests were failing. Not sure if you want to try or assign to @jonathanolson to get that one done.
There are around 12 references remaining to http://phet.colorado.edu, but I was uncomfortable changing them because they were things like the update check URL or build process steps, and I really didn't want to break these. You can change, leave, or reassign based on your discretion.

Back to @samreid for next steps.

samreid commented 3 years ago

I updated some other repos I was comfortable with, but I see about 11 remaining (may be different than yours due to which directories are excluded from the IDE). But these 11 seem like they require @jonathanolson to determine whether they can be changed or not. @jonathanolson can you please review the remaining occurrences of http://phet.colorado.edu in master and see which can be safely converted to https?

jonathanolson commented 3 years ago

@mattpen thoughts on the following URL generated? It doesn't seem to be used currently:

http://phet.colorado.edu/html-sim-update?simulation=${encodeURIComponent( simName )}&version=${encodeURIComponent( simVersion.toString() )}&buildTimestamp=${encodeURIComponent(${phet.chipper.buildTimestamp})};

mattpen commented 3 years ago

I reviewed the Apache httpd conf files, the phet document root, the wicket repo and the react/meteor repo. I didn't find any places where we'd be serving a response to //phet.colorado.edu/html-sim-update, whether it is http or ssl. Code using that path is probably broken or not used.

jbphet commented 3 years ago

A bunch of my commits initially failed to push for this issue, not sure why. I've pushed them now.

zepumph commented 1 year ago

I saw a couple more usages, and no problems with kite unit tests.

@jonathanolson there are two that feel very scary for me to change. Can you please take a look?

Subject: [PATCH] api generate for getPhetioElementState, https://github.com/phetsims/phet-io/issues/1975
---
Index: joist/js/updateCheck.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/updateCheck.ts b/joist/js/updateCheck.ts
--- a/joist/js/updateCheck.ts   (revision d1f84d632889238e6e5c6ed0bde4c7bc8f298644)
+++ b/joist/js/updateCheck.ts   (date 1700260240870)
@@ -45,7 +45,7 @@
     // If it's not PhET-branded OR if it is phet-io or in the phet-app, do not check for updates
     this.areUpdatesChecked = phet.chipper.brand === 'phet' && !phet.chipper.isApp;

-    this.updateURL = `${'http://phet.colorado.edu/html-sim-update' +
+    this.updateURL = `${'https://phet.colorado.edu/html-sim-update' +
                         '?simulation='}${encodeURIComponent( simName )
     }&version=${encodeURIComponent( simVersion.toString() )
     }&buildTimestamp=${encodeURIComponent( `${phet.chipper.buildTimestamp}` )}`;
Index: yotta/js/apacheParsing.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/yotta/js/apacheParsing.js b/yotta/js/apacheParsing.js
--- a/yotta/js/apacheParsing.js (revision 4dc84716d0d1575be7d3a4e3bf5e16c644240c1d)
+++ b/yotta/js/apacheParsing.js (date 1700260240875)
@@ -435,7 +435,7 @@
 assert( lineData5.userAgent === undefined );

 // quote in request URL (bah)
-const lineData6 = apacheParsing.parseLine( apacheParsing.figaroRegularRegex, '1.2.3.4 1.2.3.4 - - [27/Apr/2014:04:31:36 -0600] "GET /blog/2013/10/17/have-your-tried-the-html5-molarity-simulation/\\" HTTP/1.1" 404 4812 "http://phet.colorado.edu/" "Mozilla/5.0 (Linux; U; Android 2.2; fr-fr; Desire_A8181 Build/FRF91) App3leWebKit/53.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1" 0' );
+const lineData6 = apacheParsing.parseLine( apacheParsing.figaroRegularRegex, '1.2.3.4 1.2.3.4 - - [27/Apr/2014:04:31:36 -0600] "GET /blog/2013/10/17/have-your-tried-the-html5-molarity-simulation/\\" HTTP/1.1" 404 4812 "https://phet.colorado.edu/" "Mozilla/5.0 (Linux; U; Android 2.2; fr-fr; Desire_A8181 Build/FRF91) App3leWebKit/53.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1" 0' );
 assert( lineData6.method === 'GET' );
 assert( lineData6.statusCode === '404' );
 assert( lineData6.timestamp === '27/Apr/2014:04:31:36 -0600' );
@@ -443,7 +443,7 @@
 assert( lineData6.userAgent === 'Mozilla/5.0 (Linux; U; Android 2.2; fr-fr; Desire_A8181 Build/FRF91) App3leWebKit/53.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1' );

 // quote in request URL (bah)
-const lineData7 = apacheParsing.parseLine( apacheParsing.figaroRegularRegex, 'unknown, 1.2.3.4 1.2.3.4 - - [27/Apr/2014:07:38:51 -0600] "GET /blog/2013/06/21/phet-in-the-philippines/ HTTP/1.1" 200 7988 "http://phet.colorado.edu/blog/2013/06/21/phet-in-the-philippines/#comment-627056+Result:+chosen+nickname Result: chosen nickname \\"AvefeBeepolla\\"; success (from first page);" "Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.76 Safari/537.36" 0' );
+const lineData7 = apacheParsing.parseLine( apacheParsing.figaroRegularRegex, 'unknown, 1.2.3.4 1.2.3.4 - - [27/Apr/2014:07:38:51 -0600] "GET /blog/2013/06/21/phet-in-the-philippines/ HTTP/1.1" 200 7988 "https://phet.colorado.edu/blog/2013/06/21/phet-in-the-philippines/#comment-627056+Result:+chosen+nickname Result: chosen nickname \\"AvefeBeepolla\\"; success (from first page);" "Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.76 Safari/537.36" 0' );
 assert( lineData7.method === 'GET' );
 assert( lineData7.statusCode === '200' );
 assert( lineData7.timestamp === '27/Apr/2014:07:38:51 -0600' );
@@ -459,7 +459,7 @@
 assert( lineData8.userAgent === '-' );

 // empty user agent
-const lineData9 = apacheParsing.parseLine( apacheParsing.figaroRegularRegex, '1.2.3.4 1.2.3.4 - - [27/Apr/2014:23:09:26 -0600] "GET /autocomplete?l=en&q=de HTTP/1.1" 200 49 "http://phet.colorado.edu/" "" 0' );
+const lineData9 = apacheParsing.parseLine( apacheParsing.figaroRegularRegex, '1.2.3.4 1.2.3.4 - - [27/Apr/2014:23:09:26 -0600] "GET /autocomplete?l=en&q=de HTTP/1.1" 200 49 "https://phet.colorado.edu/" "" 0' );
 assert( lineData9.method === 'GET' );
 assert( lineData9.statusCode === '200' );
 assert( lineData9.timestamp === '27/Apr/2014:23:09:26 -0600' );

jonathanolson commented 4 months ago

@zepumph I applied the html-sim-update link change above (it is just opened in a pop-up, should be safe to always have HTTPS).

The other three are in unit tests, so I'd like to not change those (as they are importantly historically correct).

@zepumph anything else to do here?

phetsims / tasks

projectURL uses http and https inconsistently for phet.colorado.edu references #1089