Project

General

Profile

Actions

Bug #2564

closed

strtol() usage

Added by Olaf-van-der-Spek almost 11 years ago. Updated almost 9 years ago.

Status:
Fixed
Priority:
Normal
Category:
-
Target version:
ASK QUESTIONS IN Forums:

Description

Lighttpd is using strtol() and atoi() to parse numbers, but I think it's not properly checking for errors.

For example:
server.port = " 80" // good
server.port = "80 " // bad

The code isn't checking for range errors. You might want to use a simple strtol() wrapper to fix this.

Actions #1

Updated by darix almost 11 years ago

use

server.port = 80

Actions #2

Updated by Olaf-van-der-Spek almost 11 years ago

darix wrote:

use [...]

I know, just pointing out an inconsistency.

Actions #3

Updated by gstrauss almost 9 years ago

While changes such as this are low priority, consistency is good.

Submitted pull request https://github.com/lighttpd/lighttpd1.4/pull/26

Actions #4

Updated by stbuehler almost 9 years ago

  • Target version set to 1.4.x

The question is whether surrounding whitespace should actually be rejected - and given that we accepted it for some time I'd rather not change that.

Actions #5

Updated by gstrauss almost 9 years ago

Would you accept the patch if I modify the patch hunk for configparser.y to issue a warning instead of an error? Checking strtol() for errors is not a bad thing. Whether or not to propagate the error is the question, and you have shared that you prefer not change existing behavior to propagate the error for config parsing. There are other uses of strtol() where some extra checks can improve robustness of the code.

Actions #6

Updated by gstrauss almost 9 years ago

Updated https://github.com/lighttpd/lighttpd1.4/pull/26 to change config parsing strtol() error to a warning.

Actions #7

Updated by stbuehler almost 9 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset r3122.

Actions #8

Updated by stbuehler almost 9 years ago

  • Target version changed from 1.4.x to 1.4.40
Actions

Also available in: Atom