Fixed issue that caused an entry with an hostname containing '-' to be invalid

gforcada / haproxy_log_analysis

HAProxy log analyzer

https://pypi.org/project/haproxy_log_analysis

GNU General Public License v3.0

88 stars 35 forks source link

Fixed issue that caused an entry with an hostname containing '-' to be invalid #8

Closed mafonso closed 9 years ago

mafonso commented 9 years ago

Fixed issue that caused an entry with an hostname containing '-' (such as server-01) to result in an invalid line.

coveralls commented 9 years ago

Coverage decreased (-0.38%) when pulling 31be3be759d3c19d800260327f21a29350f75c9b on hangaspt:fix_hostname_parsing into 1bf68080dc65eadc6bdd797a4b237cfe3438e96e on gforcada:master.

gforcada commented 9 years ago

Thanks for reporting!

Two things regarding your patch:

would you mind providing a test showing that a hostname with a dash renders the line invalid?
instead of replacing \w+ by a "catch all" .*, a [\w-]+ would be much desirable

mafonso commented 9 years ago

You're right. The "catch al"l was a bit of a lazy aproach just to get me running. And it would match other possible chars that could envetualy also be present, like an underscore. Your aproach is safer.

For compleness it turns out that by at RFC 1123 (that extends RFC 952) states that the hostname has to begin with letter or digit folowed by letter digit or hyphen and end in letter or digit. No other chars are valid.

Something like [\w]+[\w-]?[\w]+ if you don't want go on pedantic mode about length restrictions.

gforcada commented 9 years ago

That's even better!

Would you mind updating your commit with this new approach and adding some tests?

If you really don't feel like writing tests that's fine too.

gforcada commented 9 years ago

@hangaspt ping :)

gforcada commented 9 years ago

@hangaspt ping :)

gforcada commented 9 years ago

PR #11 did exactly the same as here, so closing. Reopen if you feel like it.