The code in cookies.c would arrange that c->path would always contain
a string ending in "/". This may have been an attempt to make it
easier to do a proper subpath check in is_path_prefix.
Howver, the overall result is wrong in the case
Set-Cookie: ....;path=/some/thing
and then later
http://site.example.com/some/thing
c->path gets set to "/some/thing/" which doesn't pass the test in
is_path_prefix.
The precise required algorithm is described in RFC6265 5.1.4. The
existing code fails to implement the first of the three bulleted
conditions at the end of 5.1.4.
The trailing "/" is actually not so helpful for this. It is more
convenient to change is_path_prefix to do subpath matching directly:
we change it to insist that the supposed path prefix is a textual
prefix of the request path, *and* that this happens at a path segment
boundary: ie at '/' or end of string.[1]
Accordingly, we no longer add "/" to the cookie path. When we strip
the final path element we strip the "/" too. We still insert a "/" if
the path was empty.
[1] It is not 100% clear to me what "path" (URI_PATH) is but I think
it does not include any query parameters. If I am wrong about that
then '?' should be tolerated too.
CC: Mark Wooding <mdw@distorted.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Weak points:
- alignof
- js problems
Todo:
- make js work with C++ and mozjs-17
- then mozjs-24
- then mozjs-52
- then mozjs-60
- decrease number of warnings
INIT_OPTION used to initialize union option_value at compile time by
casting the default value to LIST_OF(struct option) *, which is the
type of the first member. On sparc64 and other big-endian systems
where sizeof(int) < sizeof(struct list_head *), this tended to leave
option->value.number as zero, thus messing up OPT_INT and OPT_BOOL
at least. OPT_LONG however tended to work right.
This would be easy to fix with C99 designated initializers,
but doc/hacking.txt says ELinks must be kept C89 compatible.
Another solution would be to make register_options() read the
value from option->value.tree (the first member), cast it back
to the right type, and write it to the appropriate member;
but that would still require somewhat dubious conversions
between integers, data pointers, and function pointers.
So here's a rather more invasive solution. Add struct option_init,
which is somewhat similar to struct option but has non-overlapping
members for different types of values, to ensure nothing is lost
in compile-time conversions. Move unsigned char *path from struct
option_info to struct option_init, and replace struct option_info
with a union that contains struct option_init and struct option.
Now, this union can be initialized with no portability problems,
and register_options() then moves the values from struct option_init
to their final places in struct option.
In my x86 ELinks build with plenty of options configured in, this
change bloated the text section by 340 bytes but compressed the data
section by 2784 bytes, presumably because union option_info is a
pointer smaller than struct option_info was.
(cherry picked from elinks-0.12 commit e5f6592ee2)
Conflicts:
src/protocol/fsp/fsp.c: All options had been removed in 0.13.GIT.
src/protocol/smb/smb2.c: Ditto.
Documentation strings of most options used to contain a "\n" at the
end of each source line. When the option manager displayed these
strings, it treated each "\n" as a hard newline. On 80x24 terminals
however, the option description window has only 60 columes available
for the text (with the default setup.h), and the hard newlines were
further apart, so the option manager wrapped the text a second time,
resulting in rather ugly output where long lones are interleaved with
short ones. This could also cause the text to take up too much
vertical space and not fit in the window.
Replace most of those hard newlines with spaces so that the option
manager (or perhaps BFU) will take care of the wrapping. At the same
time, rewrap the strings in source code so that the source lines are
at most 79 columns wide.
In some options though, there is a list of possible values and their
meanings. In those lists, if the description of one value does not
fit in one line, then continuation lines should be indented. The
option manager and BFU are not currently able to do that. So, keep
the hard newlines in those lists, but rewrap them to 60 columns so
that they are less likely to require further wrapping at runtime.
There were conflicts in src/document/css/ because 0.12.GIT switched
to LIST_OF(struct css_selector) and 0.13.GIT switched to struct
css_selector_set. Resolved by using LIST_OF(struct css_selector)
inside struct css_selector_set.
straconcat reads the args with va_arg(ap, const unsigned char *),
and the NULL macro may have the wrong type (e.g. int).
Many places pass string literals of type char * to straconcat. This
is in principle also a violation, but I'm ignoring it for now because
if it becomes a problem with some C implementation, then so will the
use of unsigned char * with printf "%s", which is so widespread in
ELinks that I'm not going to try fixing it now.
Don't cast function pointers; calling functions via pointers of
incorrect types is not guaranteed to work. Instead, define the
functions with the desired types, and make them cast the incoming
parameters. Or define wrapper functions if the return types don't
match.
really_exit_prog wasn't being used outside src/dialogs/menu.c,
and I had to change its parameter type, so it's now static.
The previous code just printed time_t directly with "%ld". Now it
instead first casts to time_print_T (currently long) and then formats
with TIME_PRINT_FORMAT (currently "ld"). So the varargs will now
always match with the format string, even if time_t is longer than
long. This still doesn't correctly format time_t values larger than
LONG_MAX, though. But now it is at least easier to find some of the
places that need to be changed to support that.
I located these time_t-to-string conversions by searching for
str_to_time_t, expires, and last_visit. There are still more places
that assume every interesting time_t value fits either in 32 bits or
in a long, e.g. in the cookie editor and in the ECMAScript interface.
Inspired by bug 6.
All cookies are now constructed with the new function init_cookie.
Requested by Miciah Dashiel Butler Masters.
This also fixes a bug where the "Add cookie" button left cookie->path == NULL,
causing a crash later when deciding whether to send the cookie to the server.
Mostly this makes set_cookie more complex, as it now distinguishes
between HEADER_PARAM_NOT_FOUND and HEADER_PARAM_OUT_OF_MEMORY, and kills the cookie in the
latter case. However, the cookie->secure check became simpler.
Its return value is now an enum that lets callers know whether an
error occurred. However, this commit changes the callers only
minimally, so they do not yet check the return value.
Also, add the server in the done_handler_T of the "OK" button,
rather than in the widget_handler_T of the "Server name" field
(which check_dialog calls). This change implies that the dialog
is closed even if an error occurs while adding the server, but
that seems to be customary elsewhere in ELinks too.
src/cookies/dialogs.c (set_server): Set up a struct cookie and pass it
to accept_cookie, instead of calling set_cookie which might ask the user
to confirm. (This change also removes a dubious use of struct uri.)
src/cookies/cookies.c (get_cookie_server): No longer static.
src/cookies/cookies.h (get_cookie_server): Declare.
With regular comments in the definition of the structure itself,
and with xgettext:c-format comments in constants of that type,
if xgettext would otherwise guess wrong; so that translators
will know they'll have to double any percent signs they add.
I didn't regenerate PO files, though.