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.

Sign up for GitHub

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

Jump to bottom

fix: validate downloaded node #20821

Merged
merged 5 commits into from
Jan 10, 2025
Merged

fix: validate downloaded node #20821

merged 5 commits into from
Jan 10, 2025

Conversation

caalador
Copy link
Contributor

@caalador caalador commented Jan 9, 2025

Validate downloaded node
against the provided
sha256 hash.

Fixes #20661

@caalador
fix: validate downloaded node
55e51e4 
Validate downloaded node
against the provided
sha256 hash.

Fixes #20661
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2025 •
edited
Loading

Test Results

1 159 files  ± 0  1 159 suites  ±0   1h 33m 19s ⏱️ + 4m 13s
7 582 tests ± 0  7 526 ✅ + 1  56 💤 ±0  0 ❌ ±0 
7 952 runs  +68  7 887 ✅ +67  65 💤 +2  0 ❌  - 1 

Results for commit 180cb5e. ± Comparison against base commit 7b1e2ac.

♻️ This comment has been updated with latest results.

mcollovati
mcollovati requested changes Jan 9, 2025
View reviewed changes
flow-server/src/main/java/com/vaadin/flow/internal/MessageDigestUtil.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
mcollovati
mcollovati requested changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@mcollovati mcollovati left a comment

Choose a reason for hiding this comment

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

I proposed some additional minor changes.
The only potential "blocker" is the missing Javadoc in MessageDigestUtil.
Other than that, tested locally and it works great!

flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/VerificationException.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/VerificationException.java Outdated Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/installer/NodeInstaller.java Outdated Show resolved Hide resolved
@caalador
Add and update javadoc
180cb5e 
Update log messages
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
11 New issues
9 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@mcollovati mcollovati merged commit fb0862f into main Jan 10, 2025
26 checks passed
@mcollovati mcollovati deleted the issues/20661-verify-node-download branch January 10, 2025 08:40
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.7.0.alpha4 and is also targeting the upcoming stable 24.7.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollovati mcollovati mcollovati approved these changes

Assignees
No one assigned
Labels
prerelease version for Vaadin 24.7.0 Released with Vaadin 24.7.0.alpha4 +0.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NodeInstaller skips node download when an empty bundle is present
3 participants
@caalador @vaadin-bot @mcollovati