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

feat(output): auto-open HTML file after scan #1412

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hogo6002
Copy link
Contributor

@hogo6002 hogo6002 commented Nov 20, 2024

Added support to automatically open the HTML file in users' default browser after a scan finishes.

Implementing this by using the right open command for each OS (like xdg-open for Linux, start for Windows, and open for macOS). Simultaneously, the file is served on localhost for easy remote access.

@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 31 lines in your changes missing coverage. Please review.

Project coverage is 69.43%. Comparing base (5e82cf7) to head (686493a).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
cmd/osv-scanner/scan/main.go 0.00% 30 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1412      +/-   ##
==========================================
+ Coverage   69.05%   69.43%   +0.38%     
==========================================
  Files         186      186              
  Lines       18237    18357     +120     
==========================================
+ Hits        12593    12746     +153     
+ Misses       4965     4934      -31     
+ Partials      679      677       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@hogo6002
Copy link
Contributor Author

Hmm, thinking a bit more, if we host it on a localhost port, users could potentially use SSH port forwarding to access the website if they are remotely accessing the server. But I don't know how likely this case would be.
ssh -L local_port:destination_server_ip:remote_port ssh_server_hostname

@oliverchang
Copy link
Collaborator

Hmm, thinking a bit more, if we host it on a localhost port, users could potentially use SSH port forwarding to access the website if they are remotely accessing the server. But I don't know how likely this case would be. ssh -L local_port:destination_server_ip:remote_port ssh_server_hostname

This is the workflow I typically use whenever I need to inspect a web page generated by some tool (e.g. documentation)

@hogo6002
Copy link
Contributor Author

This is the workflow I typically use whenever I need to inspect a web page generated by some tool (e.g. documentation)

Updated code to also host the HTML file on localhost

// This may be nil.
return r, err
}

// openHTML opens the outputted HTML file and simultaneously hosts it on localhost:8000 for remote access.
func openHTML(outputPath string, r reporter.Reporter) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: r should be the first argument

// This could be done in a separate goroutine to avoid blocking the main thread,
// but since this is basically the last step in generating the HTML output,
// it's kept in the main thread for better maintainability and readability.
servePort := "8000"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably only be done when the open command fails.

Copy link
Contributor Author

@hogo6002 hogo6002 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is primarily for users to remote access.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming open will most likely fail if it's a machine that is being sshed into, which is why I'm thinking this should start the server when xdg-open fails.

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a flag --host or similar where we assume output is a temporary file, format is html, and run the hosting part of the code

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.

4 participants