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

fix: fixes log parser #3398

Merged
merged 1 commit into from
Nov 16, 2024
Merged

fix: fixes log parser #3398

merged 1 commit into from
Nov 16, 2024

Conversation

pilat
Copy link
Contributor

@pilat pilat commented Nov 16, 2024

Hello, @amir20 and thank you for creating Dozzle! It’s a fantastic tool, and I really appreciate the effort you’ve put into developing it.

I recently encountered a crash with the following panic:

panic: runtime error: slice bounds out of range [222:221]

goroutine 260 [running]:
github.com/amir20/dozzle/internal/docker.ParseLogFmt({0x400060751f, 0x18c})
    /dozzle/internal/docker/logfmt.go:23 +0x3f8
github.com/amir20/dozzle/internal/docker.createEvent({0x4000607500?, 0x1ac}, 0x113a7d0?)
    /dozzle/internal/docker/event_generator.go:198 +0x50c
github.com/amir20/dozzle/internal/docker.(*EventGenerator).consumeReader(0x4000319ce0)
    /dozzle/internal/docker/event_generator.go:95 +0x50
created by github.com/amir20/dozzle/internal/docker.NewEventGenerator in goroutine 215
    /dozzle/internal/docker/event_generator.go:54 +0x260

The issue seems to originate in ParseLogFmt, where a slice bounds error causes the application to crash.

In this PR, I’m proposing a simple fix to avoid this panic. Additionally, I’ve added a test to cover the corner case that leads to the panic to help prevent similar issues in the future. If you have any feedback or need adjustments, please let me know, and I’d be happy to make changes.

@amir20
Copy link
Owner

amir20 commented Nov 16, 2024

That’s incredible! I’ll review it today. I mostly implemented LogFmt using ChatGPT. Haha!

@amir20 amir20 merged commit 80d502e into amir20:master Nov 16, 2024
7 checks passed
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