Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

send cookies via secure channel #703

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Conversation

jukzi
Copy link
Contributor

@jukzi jukzi commented Sep 20, 2023

@akurtakov
Copy link
Member

Please verify that the help system still works with this change. The code and ideas there are pretty ancient so bigger changes might be needed (hopefully not) .

@github-actions
Copy link
Contributor

Test Results

       42 files         42 suites   52m 55s ⏱️
  3 775 tests   3 772 ✔️   3 💤 0
11 328 runs  11 301 ✔️ 27 💤 0

Results for commit 06d9d3e.

@jukzi
Copy link
Contributor Author

jukzi commented Sep 20, 2023

Please verify that the help system still works with this change. The code and ideas there are pretty ancient so bigger changes might be needed (hopefully not) .

The code is called in for example org.eclipse.ua.tests.help.search.InfocenterWorkingSetManagerTest.testIWSMWithTwoTopics() and the test does not fail. Does anybody have other ideas how to test? I don't even know where it is used in real live.

@iloveeclipse
Copy link
Member

Does anybody have other ideas how to test? I don't even know where it is used in real live.

@howlger may know.

@howlger
Copy link
Contributor

howlger commented Sep 21, 2023

Does anybody have other ideas how to test? I don't even know where it is used in real live.

@howlger may know.

I don't know how to test this automatically. The following functions might be affected by the changes:

  • Active help, e.g. in Help > Help Contents: Eclipse Platform User Guide > Getting started > Basic tutorial > The Workbench clicking the link "Help > Welcome" should do the same as selecting the menu Help > Welcome
  • Persistence, e.g. a set search scope (with the link "Scope" to the right of the search field) should still be there after a restart
  • Persistence in the Information Center, e.g. a set search scope should still be there after restarting the web browser

Ideally, this should be tested in different operating systems and with different web browsers.

@jukzi
Copy link
Contributor Author

jukzi commented Sep 25, 2023

@howlger i do not understand how to test this before submitting it, as the webpage seems to be hosted on https://help.eclipse.org/

@howlger
Copy link
Contributor

howlger commented Sep 25, 2023

@howlger i do not understand how to test this before submitting it, as the webpage seems to be hosted on https://help.eclipse.org/

The so-called Information Center can be run as a plain, stand-alone Java application as described in the section How to start or stop information center from command line:

java -classpath d:\myApp\eclipse\plugins\org.eclipse.help.base_[version].jar org.eclipse.help.standalone.Infocenter -command start -eclipsehome d:\myApp\eclipse -port 8081

This will run the webpage locally at http://127.0.0.1:8081/help/index.jsp (similar to https://help.eclipse.org/ but with less content).

@jukzi
Copy link
Contributor Author

jukzi commented Sep 25, 2023

I restarted both Infocenter and the webbrowser. After restart the scope is still the configured one:
image
so it seems to work. - even tough http://127.0.0.1:8081/help/index.jsp is not https.

@howlger
Copy link
Contributor

howlger commented Sep 25, 2023

Thanks for testing.

+1 for merging assuming active help also still works. It would be great to have this security improvement in M1.

@jukzi jukzi merged commit ca35cee into eclipse-platform:master Sep 25, 2023
14 checks passed
@jukzi jukzi deleted the cookie branch September 25, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants