[Toybox] [PATCH] wget: do not append toybox version at runtime

[Rob Landley]

On 7/5/20 1:19 AM, Ariadne Conill wrote:
> The sprintf() call, while technically valid (17 bytes fits in an 18
> byte allocation) trips Alpine fortify-headers due to checking for
> allocations that could potentially overrun.

I admit it's non-ideal code, that's why it's in pending. Why does alpine build
toybox commands out of pending? They're not guaranteed to work. Heck, they're
not guaranteed to _compile_.

The toys/pending directory is more or less our version of linux/drivers/staging
except with a lower bar to be in there. (There's a README in the directory about
this: pending being empty and getting removed is one of the prerequisites for
toybox's 1.0 release.) I have very high standards for completed toybox commands,
and will happily (try my best to) fix any issue you have with them. But just
about anything that's still in pending is there because it's a can of worms to fix.

This particular command needs a surprising amount of work for its size. Here's
the first line of non-declaration non-comment code in wget_main():

  if (!(toys.optflags & FLAG_O)) help_exit("no filename");

1) the modern test is if (!FLAG(O))

2) The optargs string has | to indicate an option is required and lib/args.c
will error for you if it isn't supplied, so this line can go away and be
replaced with literally a single character in the command description.

3) requiring -O is silly, it should be able to get it from the URL and/or the
redirect. The design is wrong and missing a chunk of plumbing to do that.

So that's three things wrong with the first line of code, from trivial "there's
a convenience macro to make this more readable" to "submitter didn't know how to
use the available infrastructure" to "what it's trying to accomplish is wrong".

When I do find a large block of time to sit down and focus on this, I'd probably
want to write httpd at the same time because I need to come up to speed on what
all the funky "headers:" the server and client exchange are/mean (I started
doing that once but got pulled away) and that domain expertise is the same at
sending and receiving ends so might as well implement both while it's fresh in
my mind. (And then I need to come up with tests for interrupting and resuming
partial downloads and so on.)

And some of the headers are for things like sending gzipped data through these
connections which I have deflate infrastructure for in toybox already (but I
need to circle back around to writing the gzip compression side, which is
currently a stub because I hit the issue "if I want to be binary compatible with
gzip output, when do you perform dictionary flushes and when do you use the
builtin dictionary instead of saving one?" which would be a lot easier to answer
if I let myself read the other implementation's source code but I try to avoid
doing for licensing reasons even when it ISN'T gnu code I try to avoid reading
for headache/nausea/rage reasons; I should break down and read the old busybox
1.2.1 code for this since I've already _read_ that as the project's maintainer
and can't really be _more_ contaminated there now can I).

There's a bunch of other issues, what do I have bookmarked in my todo list...

http://lists.landley.net/pipermail/toybox-landley.net/2016-February/008006.html

Yes, cleanup for this command has a dependency graph.

At the moment, I'm trying hard to finish the new shell implementation before
opening another can of worms. It's big and complicated and I put it off a LONG
time because I knew it would eat my life for a while. I believe I'm more than
halfway through it, but how _much_ more I couldn't tell you...

> The call is pointless anyway -- as we are appending a constant to
> another constant, it is better to just let the compiler do so and
> calculate the size.  This is supported by ISO C89 and later, and
> thus any compiler that would be used to compile toybox.

I applied the patch, but given the above context, the question "are you actually
using these commands on alpine"...?

I mean, good luck if you are. Android's using quite a few itself:

$ grep pending aosp/external/toybox/Android.bp
    # Edit the relevant `srcs` below, depending on where the toy should be
    "toys/pending/dd.c",
    "toys/pending/diff.c",
    "toys/pending/expr.c",
    "toys/pending/getopt.c",
    "toys/pending/tr.c",
    "toys/pending/getfattr.c",
    "toys/pending/lsof.c",
    "toys/pending/modprobe.c",
    "toys/pending/more.c",
    "toys/pending/readelf.c",
    "toys/pending/stty.c",
    "toys/pending/traceroute.c",
    "toys/pending/vi.c",

But the ones android's using are my priority to clean up (and most of them are
in better shape with a lot less work required, more auditing than reconstructions).

There's a reason the build issues a red line warning for using stuff out of the
pending directory, and "make defconfig" does not include anything from pending.

Rob

P.S. You'll have to imagine the "gomenasai" and kowtow for being perpetually
slow at getting everything done. I am aware I suck, while having very high
standards. Ira Glass implied one grows out of this
(https://www.youtube.com/watch?v=91FQKciKfHI) but so far...