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

Check for stoi out_of_range for FITS header parsing #1369

Merged
merged 2 commits into from
May 6, 2024
Merged

Check for stoi out_of_range for FITS header parsing #1369

merged 2 commits into from
May 6, 2024

Conversation

pford
Copy link
Collaborator

@pford pford commented Apr 16, 2024 •
edited by kswang1029
Loading

Description

  • What is implemented or fixed? Mention the linked issue(s), if any.
    Fixes Backend crashes when loading a fits-gz file #1366
  • How does this PR solve the issue? Give a brief summary.
    Catch std::out_of_range exception from std::stoi and use std::stol instead.
  • Are there any companion PRs (frontend, protobuf)?
    No
  • Is there anything else that testers should know (e.g. exactly how to reproduce the issue)?
    FITS gz image attached in issue. Open without error or crash.

Checklist

  • changelog updated / no changelog update needed
  • e2e test passing / corresponding fix added / new e2e test created
  • ICD test passing / corresponding fix added / new ICD test created
  • protobuf updated to the latest dev commit / no protobuf update needed
  • protobuf version bumped / protobuf version not bumped
  • added reviewers and assignee
  • added ZenHub estimate, milestone, and release

@pford pford added the awaiting testing For pull requests that require testing label Apr 16, 2024
@pford pford added this to the v5.0-beta milestone Apr 16, 2024
@github-actions GitHub Actions
Copy link

Code Coverage

Package Line Rate Health
src.Cache 72% ➖
src.DataStream 44% ➖
src.FileList 67% ➖
src.Frame 36% ❌
src.HttpServer 42% ➖
src.ImageData 27% ❌
src.ImageFitter 83% ✔
src.ImageGenerators 43% ➖
src.ImageStats 75% ✔
src.Logger 37% ❌
src.Main 52% ➖
src.Region 69% ➖
src.Session 4% ❌
src.Table 52% ➖
src.ThreadingManager 67% ➖
src.Timer 85% ✔
src.Util 40% ➖
Summary 46% (8605 / 18783) ➖

kswang1029
kswang1029 approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

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

looks good. no regression from e2e.

@confluence confluence merged commit 5d7d057 into dev May 6, 2024
14 checks passed
@confluence confluence deleted the pam/1366_fz_header_crash branch May 6, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kswang1029 kswang1029 kswang1029 approved these changes

@confluence confluence confluence approved these changes

@loveluthien loveluthien Awaiting requested review from loveluthien

Assignees

@confluence confluence

Labels
awaiting testing For pull requests that require testing
Projects
None yet
Milestone
v5.0-beta
Development

Successfully merging this pull request may close these issues.

Backend crashes when loading a fits-gz file
3 participants
@pford @confluence @kswang1029