Re: [PATCH v6 3/7] setup: sanity check file size in read_gitfile_gently

From: erik elfström <erik.elfstrom@gmail.com>
Date: May 12, 2015, 06:46 UTC
Message-ID: <CAMpP7NYEqTeVm3ybPmV7TvRkj5MjRYvD0J76ezXi9_jQsFterg@mail.gmail.com>
In-Reply-To: <1431288041-21077-4-git-send-email-erik.elfstrom@gmail.com>

On Sun, May 10, 2015 at 10:00 PM, Erik Elfström <erik.elfstrom@gmail.com> wrote:

Show 9 quoted lines

> @@ -404,6 +405,11 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
>                 error_code = READ_GITFILE_ERR_OPEN_FAILED;
>                 goto cleanup_return;
>         }
> +       if (st.st_size > one_MB) {
> +               close(fd);
> +               error_code = READ_GITFILE_ERR_TOO_LARGE;
> +               goto cleanup_return;
> +       }

Hmm... The order should probably be changed here. It would make more sense to check the size before opening the file. That way the error handling in clean would be more consistent if we can't open a large .git file.

Right now we would treat any file that we can't open as a potential repository and avoid cleaning but if we can open it and it turns out that it is larger than 1MB we will ignore it and clean. By switching the order here we would always ignore files larger than 1MB regardless of if we can open them or not and I think that would make more sense. It would also remove the need to close the file when erroring out due to size so it makes more sense from a pure structural point of view as well.

Sorry for not thinking of this earlier.

/Erik