Merge pull request #4521 from pks-t/pks/config-crlf-lines

config: handle CRLF-only lines and BOM

src/buf_text.c

@@ -188,7 +188,7 @@ bool git_buf_text_is_binary(const git_buf *buf)
 	git_bom_t bom;
 	int printable = 0, nonprintable = 0;
 
-	scan += git_buf_text_detect_bom(&bom, buf, 0);
+	scan += git_buf_text_detect_bom(&bom, buf);
 
 	if (bom > GIT_BOM_UTF8)
 		return 1;
@@ -215,18 +215,18 @@ bool git_buf_text_contains_nul(const git_buf *buf)
 	return (memchr(buf->ptr, '\0', buf->size) != NULL);
 }
 
-int git_buf_text_detect_bom(git_bom_t *bom, const git_buf *buf, size_t offset)
+int git_buf_text_detect_bom(git_bom_t *bom, const git_buf *buf)
 {
 	const char *ptr;
 	size_t len;
 
 	*bom = GIT_BOM_NONE;
-	/* need at least 2 bytes after offset to look for any BOM */
-	if (buf->size < offset + 2)
+	/* need at least 2 bytes to look for any BOM */
+	if (buf->size < 2)
 		return 0;
 
-	ptr = buf->ptr + offset;
-	len = buf->size - offset;
+	ptr = buf->ptr;
+	len = buf->size;
 
 	switch (*ptr++) {
 	case 0:
@@ -274,7 +274,7 @@ bool git_buf_text_gather_stats(
 	memset(stats, 0, sizeof(*stats));
 
 	/* BOM detection */
-	skip = git_buf_text_detect_bom(&stats->bom, buf, 0);
+	skip = git_buf_text_detect_bom(&stats->bom, buf);
 	if (skip_bom)
 		scan += skip;
 

src/buf_text.h

@@ -99,11 +99,9 @@ extern bool git_buf_text_contains_nul(const git_buf *buf);
  *
  * @param bom Set to the type of BOM detected or GIT_BOM_NONE
  * @param buf Buffer in which to check the first bytes for a BOM
- * @param offset Offset into buffer to look for BOM
  * @return Number of bytes of BOM data (or 0 if no BOM found)
  */
-extern int git_buf_text_detect_bom(
-	git_bom_t *bom, const git_buf *buf, size_t offset);
+extern int git_buf_text_detect_bom(git_bom_t *bom, const git_buf *buf);
 
 /**
  * Gather stats for a piece of text

src/config_parse.c

@@ -217,7 +217,7 @@ static int skip_bom(git_parse_ctx *parser)
 {
 	git_buf buf = GIT_BUF_INIT_CONST(parser->content, parser->content_len);
 	git_bom_t bom;
-	int bom_offset = git_buf_text_detect_bom(&bom, &buf, parser->content_len);
+	int bom_offset = git_buf_text_detect_bom(&bom, &buf);
 
 	if (bom == GIT_BOM_UTF8)
 		git_parse_advance_chars(parser, bom_offset);
@@ -475,6 +475,11 @@ int git_config_parse(
 		size_t line_len = parser->ctx.line_len;
 		char c;
 
+		/*
+		 * Get either first non-whitespace character or, if that does
+		 * not exist, the first whitespace character. This is required
+		 * to preserve whitespaces when writing back the file.
+		 */
 		if (git_parse_peek(&c, ctx, GIT_PARSE_PEEK_SKIP_WHITESPACE) < 0 &&
 		    git_parse_peek(&c, ctx, 0) < 0)
 			continue;
@@ -490,6 +495,7 @@ int git_config_parse(
 			break;
 
 		case '\n': /* comment or whitespace-only */
+		case '\r':
 		case ' ':
 		case '\t':
 		case ';':

tests/config/read.c

@@ -703,3 +703,48 @@ void test_config_read__path(void)
 	git_buf_free(&expected_path);
 	git_config_free(cfg);
 }
+
+void test_config_read__crlf_style_line_endings(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "[some]\r\n var = value\r\n");
+	cl_git_pass(git_config_open_ondisk(&cfg, "./testconfig"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "some.var"));
+	cl_assert_equal_s(buf.ptr, "value");
+
+	git_config_free(cfg);
+	git_buf_free(&buf);
+}
+
+void test_config_read__trailing_crlf(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "[some]\r\n var = value\r\n\r\n");
+	cl_git_pass(git_config_open_ondisk(&cfg, "./testconfig"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "some.var"));
+	cl_assert_equal_s(buf.ptr, "value");
+
+	git_config_free(cfg);
+	git_buf_free(&buf);
+}
+
+void test_config_read__bom(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some]\n var = value\n");
+	cl_git_pass(git_config_open_ondisk(&cfg, "./testconfig"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "some.var"));
+	cl_assert_equal_s(buf.ptr, "value");
+
+	git_config_free(cfg);
+	git_buf_free(&buf);
+}