Skip to content

Improved SHA256 Signature Extraction#409

Merged
ExtremeFiretop merged 1 commit intoExtremeFiretop:WebFunfrom
Martinski4GitHub:WebFun
Feb 18, 2025
Merged

Improved SHA256 Signature Extraction#409
ExtremeFiretop merged 1 commit intoExtremeFiretop:WebFunfrom
Martinski4GitHub:WebFun

Conversation

@Martinski4GitHub
Copy link
Collaborator

Code improvements for extracting SHA256 signatures.

Code improvements for extracting SHA256 signatures.
@ExtremeFiretop ExtremeFiretop merged commit 7f4fef2 into ExtremeFiretop:WebFun Feb 18, 2025
1 check passed
@Martinski4GitHub
Copy link
Collaborator Author

@ExtremeFiretop,

I took a closer look at the fix because I was wondering why the following line was now needed in the modified curl cmd call:

sed -e 's/SHA256 signatures:Latest release://'

It turned out it was because you changed the syntax (the dots were removed from the previous call) here:

sed -n '/<pre[^>]*>/,/<\/pre>/p'  |
sed -e 's/<[^>]*>//g'

After doing testing with the original code, I found that the root cause of the problem was not really with the curl cmd; the issue was due to the cut cmd used later to extract the signature. The fix would be to use awk instead of the cut cmd here:

dl_sig="$(echo "$checksums" | grep "$(basename "$firmware_file")" | awk -F ' ' '{print $1}')"

In the end, the better fix would be to adjust the curl and use awk cmd for extraction.

I have submitted a PR to each branch with the resulting fixes.
Please review when you get some time.

@ExtremeFiretop
Copy link
Owner

@ExtremeFiretop,

I took a closer look at the fix because I was wondering why the following line was now needed in the modified curl cmd call:

sed -e 's/SHA256 signatures:Latest release://'

It turned out it was because you changed the syntax (the dots were removed from the previous call) here:

sed -n '/<pre[^>]*>/,/<\/pre>/p'  |
sed -e 's/<[^>]*>//g'

After doing testing with the original code, I found that the root cause of the problem was not really with the curl cmd; the issue was due to the cut cmd used later to extract the signature. The fix would be to use awk instead of the cut cmd here:

dl_sig="$(echo "$checksums" | grep "$(basename "$firmware_file")" | awk -F ' ' '{print $1}')"

In the end, the better fix would be to adjust the curl and use awk cmd for extraction.

I have submitted a PR to each branch with the resulting fixes. Please review when you get some time.

Great! Thanks for double-checking and finding that the cut command was the culprit.
It's always helpful to have another pair of eyes on these kinds of issues.

I really appreciate the time you took to review and verify the fix!

@Martinski4GitHub
Copy link
Collaborator Author

@ExtremeFiretop,
I took a closer look at the fix because I was wondering why the following line was now needed in the modified curl cmd call:

sed -e 's/SHA256 signatures:Latest release://'

It turned out it was because you changed the syntax (the dots were removed from the previous call) here:

sed -n '/<pre[^>]*>/,/<\/pre>/p'  |
sed -e 's/<[^>]*>//g'

After doing testing with the original code, I found that the root cause of the problem was not really with the curl cmd; the issue was due to the cut cmd used later to extract the signature. The fix would be to use awk instead of the cut cmd here:

dl_sig="$(echo "$checksums" | grep "$(basename "$firmware_file")" | awk -F ' ' '{print $1}')"

In the end, the better fix would be to adjust the curl and use awk cmd for extraction.
I have submitted a PR to each branch with the resulting fixes. Please review when you get some time.

Great! Thanks for double-checking and finding that the cut command was the culprit. It's always helpful to have another pair of eyes on these kinds of issues.

I really appreciate the time you took to review and verify the fix!

Of course, bud. We got each other's back when reviewing our PRs. It's a team effort always!! :>).

Have a good night, bud. Sleep tight!!

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.

2 participants