Skip to content

fix(hdr): Make the HDR reader tolerant to CR in the ASCII header#5261

Merged
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-hdr-crlf
Jun 27, 2026
Merged

fix(hdr): Make the HDR reader tolerant to CR in the ASCII header#5261
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-hdr-crlf

Conversation

@lgritz

@lgritz lgritz commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Carelessly written hdr/rgbe files, or correctly written ones that are checked out by git on windows if you're not careful, might use Windows CRLF line endings. Be silently tolerant of that.

Assisted-by: Claude Code / opus-4.8

Carelessly written hdr/rgbe files, or correctly written ones that are
checked out by git on windows if you're not careful, might use Windows
CRLF line endings. Be silently tolerant of that.

Assisted-by: Claude Code / opus-4.8

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@jfpanisset

Copy link
Copy Markdown

I don't know anything about this code, but should this fix live in Strutil::parse_line() (in src/libutil/strutil.cpp) which seems to assume LF terminated strings and could generally protect itself against CR/LF strings?

@lgritz

lgritz commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

should this fix live in Strutil::parse_line()

That's a really good question! I'm not really sure, to be honest. But I'll note the following:

  • Strutil::parse_line says it will "Return the prefix of str up to and including the first newline" and that's precisely what it does, and I'm hesitant to change its behavior for fear of what else might break some day (especially in a public header full of utilities used by other projects).
  • I think of the crlf issue as one of how text is stored in text files on Windows (ha! I started to type MSDOS out of habit, maybe that is telling), and parse_line is about in-memory strings and says nothing about file I/O.
  • The code in hdrinput is very much about files on disk -- reading them into memory and then using the string parsing on a portion of the header that is ASCII, so I think probably that's where the logic belongs.
  • In 18 years of OIIO, we have never had a problem with parse_line, and here I'm addressing something specific to hdr file header parsing (particularly of subtly corrupted hdr files).

I think these add up to my keeping the change I have as local to hdr as possible, and not wanting to mess with parse_line.

But it was not an obvious answer, I really had to look at the code and think about it after pondering your question.

@jfpanisset

Copy link
Copy Markdown

Hopefully you didn't waste too much time on this... I agree with your analysis and "LGTM".

@lgritz lgritz merged commit 77906d3 into AcademySoftwareFoundation:main Jun 27, 2026
27 checks passed
@lgritz lgritz deleted the lg-hdr-crlf branch June 27, 2026 23:47
lgritz added a commit that referenced this pull request Jul 1, 2026
Carelessly written hdr/rgbe files, or correctly written ones that are
checked out by git on windows if you're not careful, might use Windows
CRLF line endings. Be silently tolerant of that.

Assisted-by: Claude Code / opus-4.8

Signed-off-by: Larry Gritz <lg@larrygritz.com>
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