labstack / gommon

Common packages for Go
MIT License
536 stars 100 forks source link

Fix panic in log with empty header #51

Closed arp242 closed 2 years ago

arp242 commented 2 years ago

Previously using l.SetHeader("") would panic with:

--- FAIL: TestEmptyHeader (0.00s)
panic: runtime error: index out of range [-1] [recovered]
    panic: runtime error: index out of range [-1]

goroutine 18 [running]:
github.com/labstack/gommon/log.(*Logger).log(0xc00014e990, 0x1, {0x5fb505, 0xd}, {0x0, 0x0, 0x0})
    /home/martin/src/gommon/log/log.go:394 +0x615
github.com/labstack/gommon/log.(*Logger).Debugf(...)
    /home/martin/src/gommon/log/log.go:158
github.com/labstack/gommon/log.TestEmptyHeader(0xc000129860?)
    /home/martin/src/gommon/log/log_test.go:128 +0xb1

This adds a check for that. It also checks if there is any content before writing a space in the "Text header" else branch so you don't end up with " my message".

codecov-commenter commented 2 years ago

Codecov Report

Merging #51 (f718471) into master (64116ba) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
+ Coverage   60.64%   60.70%   +0.06%     
==========================================
  Files           6        6              
  Lines         625      626       +1     
==========================================
+ Hits          379      380       +1     
  Misses        242      242              
  Partials        4        4              
Impacted Files Coverage Δ
log/log.go 53.15% <100.00%> (+0.21%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 64116ba...f718471. Read the comment docs.

aldas commented 2 years ago

@arp242 Thank you