And today, your default download directory is...


  • Discourse touched me in a no-no place

    I'm currently reviewing in-house binaries against a new version of the OS. Just to make sure that when things break further down the line we can blame the last person to touch it, rather than the upgrade of OS.

    Naturally I'm handed one that has no (non-code) documentation whatsoever (and the person who wrote it was downsized a while back - see Lounge.)

    So.

    Back to the canonical documentation - the code - to see what minmal configuration the rig needs in order to verify whether it works (or not.)

    One piece of 'optional' configuration is a directory that a remotely downloaded configuration file gets put into.

    Now when I say 'optional' I mean that if it's omitted from configuration it defaults to a particular directory, #defined as WG_DEF_LOCAL_PATH.

    Then we have a blacklist of directories that can't be used for such a purpose. This is verbatim from what the code reads like at the moment. The only changes I'm currently permitted to make are comments (see comment above about blame. )

    I've added one here - see if you can find it.

    Code:
    /**
     * A null terminated list of root directory entries into which it is prohibited to download data.
     * Each entry has a leading and trailing slash
     */
    char *download_prohibition_list[] = {
    
    	"/bin/",
    	"/boot/",
    	"/dev/",
    	"/etc/",
    	"/home/",
    	"/lib/",
    	"/media/",
    	"/mnt/",
    	"/opt/",
    	"/proc/",
    	"/root/",
    	"/sbin/",
    	"/srv/",
    	"/sys/",
    	"/tmp/", /* TODO: Guess what the default of WG_DEF_LOCAL_PATH is? - PJH */
    	"/usr/",
    	NULL
    };
    


  • @pjh said in And today, your default download directory is...:

    The only changes I'm currently permitted to make are comments

    0_1505146747788_Screen_shot_2012-01-18_at_5.28.23_PM.png


  • Discourse touched me in a no-no place

    In this situation, it's perfectly cromulant.

    Without (code) changes:

    • If it works, and is marked as such, any future changes that break it can be put down to the code changes.
    • If it doesn't, but did on the old OS, then the problem is with the change of OS. Either the OS needs tweaking, or the program needs a workaround.

    With (irrelevant) code changes:

    • If it works - woo hoo!
    • If it doesn't - now then; what broke it? The change or the OS?

    This is wholly investigative at this point.

    I'm just noticing (other) things during in impromptu and 'unofficial' code review (while also creating documentation) and marking them inline to be fixed later.



  • @pjh said in And today, your default download directory is...:

    Code:
    /**
     * A null terminated list of root directory entries into which it is prohibited to download data.
     * Each entry has a leading and trailing slash
     */
    char *download_prohibition_list[] = {
    
    	"/bin/",
    	"/boot/",
    	"/dev/",
    	"/etc/",
    	"/home/",
    	"/lib/",
    	"/media/",
    	"/mnt/",
    	"/opt/",
    	"/proc/",
    	"/root/",
    	"/sbin/",
    	"/srv/",
    	"/sys/",
    	"/tmp/", /* TODO: Guess what the default of WG_DEF_LOCAL_PATH is? - PJH */
    	"/usr/",
    	NULL
    };
    

    So the only directory that's allowed is /var? Or is it assumed that there's some manually-created directory?

    Also, this is a fucking terrible way to restrict where a program can create files. That's why Linux has users, groups, and permissions.

    Also also, does the program resolve symlinks before or after checking this list?



  • @pjh oh... I didn't quite understand that from the first time I read it. That makes sense.

    So you're really not allowed to change the code at all, but you're bending that rule to allow for adding some documentation as you read through it since it's not documented at all and since it'd be a shame to waste all that effort figuring out what it does and then not have some documentation as a result.


  • Java Dev

    @dragnslcr said in And today, your default download directory is...:

    Also also, does the program resolve symlinks before or after checking this list?

    Since it's C code, almost certainly after. While on the command line you have readlink to canonicalize links, there's no easy way to do so in C - you just pass the path with symlinks unresolved to the kernel and it handles matters.


  • Discourse touched me in a no-no place

    @dragnslcr said in And today, your default download directory is...:

    So the only directory that's allowed is /var? Or is it assumed that there's some manually-created directory?

    Also, this is a fucking terrible way to restrict where a program can create files. That's why Linux has users, groups, and permissions.

    Also also, does the program resolve symlinks before or after checking this list?

    Answer to both are either:

    • exercises for the student or
    • rhetorical for the denizens of this forum

    This will be rewritten. Hopefully it'll end up being me who does it.

    @anotherusername said in And today, your default download directory is...:

    So you're really not allowed to change the code at all, but you're bending that rule to allow for adding some documentation as you read through it since it's not documented at all and since it'd be a shame to waste all that effort figuring out what it does and then not have some documentation as a result.

    That's the one. Plus, including the above, it may be me changing it later.


  • Discourse touched me in a no-no place

    @pleegwat said in And today, your default download directory is...:

    there's no easy way to do so in C

    You've got access to the same API calls that readlink uses. That's what counts as “easy” with C…



  • @pleegwat said in And today, your default download directory is...:

    While on the command line you have readlink to canonicalize links, there's no easy way to do so in C

    In C you still have readlink. However if you want the equivalent of readlink -f, you have to write it yourself.


  • Discourse touched me in a no-no place

    @bulb said in And today, your default download directory is...:

    In C you still have readlink. However if you want the equivalent of readlink -f, you have to write it yourself.



  • @dkf I know. I even considered mentioning it. However, it is still non-trivial code and it involves opening the file, which you may not want to do. And falling back to opening the containing directory if the file does not exist.


  • Discourse touched me in a no-no place

    @bulb said in And today, your default download directory is...:

    it involves opening the file

    You pretty much have to do that in order to be sure that you're not going to have something swap the file out from under your feet.



  • Configuration code like this is always super buggy.

    In reality, you'll probably only ever use two values here (one in production, one in dev). So unless you have good unit test coverage, any additional settings and checks will have probably never been used after they were initially coded.


  • Java Dev

    @dkf said in And today, your default download directory is...:

    @bulb said in And today, your default download directory is...:

    In C you still have readlink. However if you want the equivalent of readlink -f, you have to write it yourself.

    That's something different, working for a already-opened file. In older linuxes, /proc/self/fd/* will only give you a device/inode pair. In newer linuxes, you can read it for the actual path you originally opened. I don't think any linux gives you the fully canonalized path, as readlink -f does.

    Consider these files:

    /u -> /a
    /a/b -> c
    /a/c -> /x
    /x
    

    opening /u/b will end up opening /x, but multiple symbolic links at multiple points along the path have to be resolved to find that location, and from C you have to write your own code for that.


Log in to reply