-
Notifications
You must be signed in to change notification settings - Fork 11
Add *MAX-LINE-LENGTH* and *MAX-HEADER-LINE-LENGTH* #9
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to what I've noted in specific comment, *max-line-length*
and *max-header-line-length*
should be added to the :export
list in [packages.lisp], and some text should be added to [docs/index.html].
read.lisp
Outdated
(let ((result | ||
(with-output-to-string (line) | ||
(loop for char-seen-p = nil then t | ||
for count from 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wacky indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you lint
read.lisp
Outdated
for char = (read-char* stream nil) | ||
for is-cr-p = (and char (char= char #\Return)) | ||
when (and *max-line-length* (> count *max-line-length*)) | ||
do (error 'input-exceeded-limit :maximum *max-line-length*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, wacky indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gefjon do you recommend any specific linter ?
UNREAD-CHAR*.") | ||
|
||
(defvar *max-line-length* nil | ||
"Maximum length of a line. If NIL, lines may be any length.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd appreciate more documentation. The docstring should address:
- what happens when attempting to read an overlong line.
- appropriate types and values this variable may take
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- appropriate types and values this variable may take
why can't compilers rely on humans to read this out of #'cl:describe-object output?
"A `buffer' for one character. Used by PEEK-CHAR* and | ||
UNREAD-CHAR*.") | ||
|
||
(defvar *max-line-length* nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add declaim
of type
for this and the other special var. I assume the correct type is (or null unsigned-byte)
, but it may be (or null (and fixnum unsigned-byte))
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(or null unsigned-byte)
, but it may be(or null (and fixnum unsigned-byte))
(cl:deftype chunga:maybe-pointer
'(or null fixnum) "I'm useless")
The point of this PR is to provably bound memory consumption, and thus the type specifier must not allow any non-fixnum values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
people rarely consider me helpful; thank you for starting the process, and you gotta believe it takes a while.
stream with input chunking enabled.")) | ||
|
||
(define-condition input-exceeded-limit (chunga-error) | ||
((maximum :initarg :maximum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should probably also get type-declared, if possible; some fascist implementations might like allocating conditions on the stack and bitch about boxed pointer safety garbage
(:report (lambda (condition stream) | ||
(with-slots (maximum) | ||
condition | ||
(format stream "Input exceeded ~d characters." maximum)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that format call will definitely be more efficient with at least one e.g. (the fixnum maximum)
(let (headers | ||
(*current-error-message* "While reading HTTP headers:")) | ||
(*current-error-message* "While reading HTTP headers:") | ||
(*max-line-length* (or *max-header-line-length* *max-line-length*))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it'll never fit in stack frames if python doesn't compute integer-length at compile time
Addresses #8.
This adds two special variables and an error condition.