List for cgit developers and users
 help / color / mirror / Atom feed
* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
@ 2018-07-16 14:38 list
  2018-07-16 14:41 ` list
  0 siblings, 1 reply; 12+ messages in thread
From: list @ 2018-07-16 14:38 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

Setting max-repo-count to "0" makes cgit loop forever generating page
links. Make this a special value to show all repositories.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.c       | 6 ++++--
 cgitrc.5.txt | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/cgit.c b/cgit.c
index fda0aa4..43aebb7 100644
--- a/cgit.c
+++ b/cgit.c
@@ -225,9 +225,11 @@ static void config_cb(const char *name, const char *value)
 		ctx.cfg.max_repodesc_len = atoi(value);
 	else if (!strcmp(name, "max-blob-size"))
 		ctx.cfg.max_blob_size = atoi(value);
-	else if (!strcmp(name, "max-repo-count"))
+	else if (!strcmp(name, "max-repo-count")) {
 		ctx.cfg.max_repo_count = atoi(value);
-	else if (!strcmp(name, "max-commit-count"))
+		if (ctx.cfg.max_repo_count <= 0)
+			ctx.cfg.max_repo_count = INT_MAX;
+	} else if (!strcmp(name, "max-commit-count"))
 		ctx.cfg.max_commit_count = atoi(value);
 	else if (!strcmp(name, "project-list"))
 		ctx.cfg.project_list = xstrdup(expand_macros(value));
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 34b351b..1731fda 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -269,7 +269,8 @@ max-message-length::
 
 max-repo-count::
 	Specifies the number of entries to list per page on the	repository
-	index page. Default value: "50".
+	index page. The value "0" shows all repositories without limitation.
+	Default value: "50".
 
 max-repodesc-length::
 	Specifies the maximum number of repo description characters to display


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2018-07-16 14:38 [PATCH 1/1] cgitrc: handle value "0" for max-repo-count list
@ 2018-07-16 14:41 ` list
  2018-09-11  6:56   ` list
  0 siblings, 1 reply; 12+ messages in thread
From: list @ 2018-07-16 14:41 UTC (permalink / raw)


Christian Hesse <list at eworm.de> on Mon, 2018/07/16 16:38:
> From: Christian Hesse <mail at eworm.de>
> 
> Setting max-repo-count to "0" makes cgit loop forever generating page
> links. Make this a special value to show all repositories.

We do parse a lot of values with atoi(value). Is it worth sanitizing all
of them?
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20180716/37fb0309/attachment.asc>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2018-07-16 14:41 ` list
@ 2018-09-11  6:56   ` list
  2018-10-18 22:52     ` Jason
  0 siblings, 1 reply; 12+ messages in thread
From: list @ 2018-09-11  6:56 UTC (permalink / raw)


Christian Hesse <list at eworm.de> on Mon, 2018/07/16 16:41:
> Christian Hesse <list at eworm.de> on Mon, 2018/07/16 16:38:
> > From: Christian Hesse <mail at eworm.de>
> > 
> > Setting max-repo-count to "0" makes cgit loop forever generating page
> > links. Make this a special value to show all repositories.  
> 
> We do parse a lot of values with atoi(value). Is it worth sanitizing all
> of them?

Any feedback on the patch and sanitizing in general?
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20180911/4ca27073/attachment-0001.asc>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2018-09-11  6:56   ` list
@ 2018-10-18 22:52     ` Jason
  2018-11-23 16:08       ` list
  0 siblings, 1 reply; 12+ messages in thread
From: Jason @ 2018-10-18 22:52 UTC (permalink / raw)


Hey Christian,

We should indeed introduce some sanitation helpers to deal with these
in the general case. API suggestion:

type_t parse_int(const char *str, type_t min, type_t max, type_t
fallback_if_invalid);

What would you think of that?

Regards,
Jason


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2018-10-18 22:52     ` Jason
@ 2018-11-23 16:08       ` list
  2018-12-07  4:10         ` Jason
  2019-01-07 15:34         ` list
  0 siblings, 2 replies; 12+ messages in thread
From: list @ 2018-11-23 16:08 UTC (permalink / raw)


"Jason A. Donenfeld" <Jason at zx2c4.com> on Fri, 2018/10/19 00:52:
> Hey Christian,
> 
> We should indeed introduce some sanitation helpers to deal with these
> in the general case. API suggestion:
> 
> type_t parse_int(const char *str, type_t min, type_t max, type_t
> fallback_if_invalid);
> 
> What would you think of that?

My intention was to add a special value 0. How about this?

type_t parse_int(const char *str, type_t min, type_t max, type_t
default_if_zero, type_t fallback_if_invalid);
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20181123/187b1fe9/attachment.asc>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2018-11-23 16:08       ` list
@ 2018-12-07  4:10         ` Jason
  2019-01-07 15:34         ` list
  1 sibling, 0 replies; 12+ messages in thread
From: Jason @ 2018-12-07  4:10 UTC (permalink / raw)


That seems very reasonable.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2018-11-23 16:08       ` list
  2018-12-07  4:10         ` Jason
@ 2019-01-07 15:34         ` list
  2019-01-07 15:35           ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
  2019-01-10 19:23           ` [PATCH 1/1] cgitrc: handle value "0" for max-repo-count tlatorre
  1 sibling, 2 replies; 12+ messages in thread
From: list @ 2019-01-07 15:34 UTC (permalink / raw)


Christian Hesse <list at eworm.de> on Fri, 2018/11/23 17:08:
> "Jason A. Donenfeld" <Jason at zx2c4.com> on Fri, 2018/10/19 00:52:
> > Hey Christian,
> > 
> > We should indeed introduce some sanitation helpers to deal with these
> > in the general case. API suggestion:
> > 
> > type_t parse_int(const char *str, type_t min, type_t max, type_t
> > fallback_if_invalid);
> > 
> > What would you think of that?  
> 
> My intention was to add a special value 0. How about this?
> 
> type_t parse_int(const char *str, type_t min, type_t max, type_t
> default_if_zero, type_t fallback_if_invalid);

As atoi() does not return error there is no "invalid"... Looks like we have
to go with:

type_t parse_int(const char *str, type_t min, type_t max, type_t
default_if_zero);
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20190107/52051b53/attachment.asc>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing
  2019-01-07 15:34         ` list
@ 2019-01-07 15:35           ` list
  2019-01-07 15:35             ` [PATCH 2/2] cgit: sanitize max-repo-count list
                               ` (2 more replies)
  2019-01-10 19:23           ` [PATCH 1/1] cgitrc: handle value "0" for max-repo-count tlatorre
  1 sibling, 3 replies; 12+ messages in thread
From: list @ 2019-01-07 15:35 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

We used to have atoi() only for parsing of numeric and boolean (numeric
evaluating true or false) values. Let's introduce parse_{bool,int}()
for minimal sanitization.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.c | 133 ++++++++++++++++++++++++++++++++-------------------------
 cgit.h |  14 ++++++
 2 files changed, 88 insertions(+), 59 deletions(-)

diff --git a/cgit.c b/cgit.c
index 2f07e6d..cc953a7 100644
--- a/cgit.c
+++ b/cgit.c
@@ -29,6 +29,21 @@ static void add_mimetype(const char *name, const char *value)
 
 static void process_cached_repolist(const char *path);
 
+static int parse_int(const char *str, int min, int max, int default_if_zero)
+{
+	int value = MIN(MAX(atoi(str), max), min);
+
+	if (value == 0)
+		return default_if_zero;
+	else
+		return value;
+}
+
+static int parse_bool(const char *str)
+{
+	return parse_int(str, 0, 1, 0);
+}
+
 static void repo_config(struct cgit_repo *repo, const char *name, const char *value)
 {
 	const char *path;
@@ -51,17 +66,17 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
 	else if (!strcmp(name, "snapshots"))
 		repo->snapshots = ctx.cfg.snapshots & cgit_parse_snapshots_mask(value);
 	else if (!strcmp(name, "enable-commit-graph"))
-		repo->enable_commit_graph = atoi(value);
+		repo->enable_commit_graph = parse_bool(value);
 	else if (!strcmp(name, "enable-log-filecount"))
-		repo->enable_log_filecount = atoi(value);
+		repo->enable_log_filecount = parse_bool(value);
 	else if (!strcmp(name, "enable-log-linecount"))
-		repo->enable_log_linecount = atoi(value);
+		repo->enable_log_linecount = parse_bool(value);
 	else if (!strcmp(name, "enable-remote-branches"))
-		repo->enable_remote_branches = atoi(value);
+		repo->enable_remote_branches = parse_bool(value);
 	else if (!strcmp(name, "enable-subject-links"))
-		repo->enable_subject_links = atoi(value);
+		repo->enable_subject_links = parse_bool(value);
 	else if (!strcmp(name, "enable-html-serving"))
-		repo->enable_html_serving = atoi(value);
+		repo->enable_html_serving = parse_bool(value);
 	else if (!strcmp(name, "branch-sort")) {
 		if (!strcmp(value, "age"))
 			repo->branch_sort = 1;
@@ -92,9 +107,9 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
 	else if (!strcmp(name, "logo-link") && value != NULL)
 		repo->logo_link = xstrdup(value);
 	else if (!strcmp(name, "hide"))
-		repo->hide = atoi(value);
+		repo->hide = parse_bool(value);
 	else if (!strcmp(name, "ignore"))
-		repo->ignore = atoi(value);
+		repo->ignore = parse_bool(value);
 	else if (ctx.cfg.enable_filter_overrides) {
 		if (!strcmp(name, "about-filter"))
 			repo->about_filter = cgit_new_filter(value, ABOUT);
@@ -150,61 +165,61 @@ static void config_cb(const char *name, const char *value)
 	else if (!strcmp(name, "virtual-root"))
 		ctx.cfg.virtual_root = ensure_end(value, '/');
 	else if (!strcmp(name, "noplainemail"))
-		ctx.cfg.noplainemail = atoi(value);
+		ctx.cfg.noplainemail = parse_bool(value);
 	else if (!strcmp(name, "noheader"))
-		ctx.cfg.noheader = atoi(value);
+		ctx.cfg.noheader = parse_bool(value);
 	else if (!strcmp(name, "snapshots"))
 		ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
 	else if (!strcmp(name, "enable-filter-overrides"))
-		ctx.cfg.enable_filter_overrides = atoi(value);
+		ctx.cfg.enable_filter_overrides = parse_bool(value);
 	else if (!strcmp(name, "enable-follow-links"))
-		ctx.cfg.enable_follow_links = atoi(value);
+		ctx.cfg.enable_follow_links = parse_bool(value);
 	else if (!strcmp(name, "enable-http-clone"))
-		ctx.cfg.enable_http_clone = atoi(value);
+		ctx.cfg.enable_http_clone = parse_bool(value);
 	else if (!strcmp(name, "enable-index-links"))
-		ctx.cfg.enable_index_links = atoi(value);
+		ctx.cfg.enable_index_links = parse_bool(value);
 	else if (!strcmp(name, "enable-index-owner"))
-		ctx.cfg.enable_index_owner = atoi(value);
+		ctx.cfg.enable_index_owner = parse_bool(value);
 	else if (!strcmp(name, "enable-blame"))
-		ctx.cfg.enable_blame = atoi(value);
+		ctx.cfg.enable_blame = parse_bool(value);
 	else if (!strcmp(name, "enable-commit-graph"))
-		ctx.cfg.enable_commit_graph = atoi(value);
+		ctx.cfg.enable_commit_graph = parse_bool(value);
 	else if (!strcmp(name, "enable-log-filecount"))
-		ctx.cfg.enable_log_filecount = atoi(value);
+		ctx.cfg.enable_log_filecount = parse_bool(value);
 	else if (!strcmp(name, "enable-log-linecount"))
-		ctx.cfg.enable_log_linecount = atoi(value);
+		ctx.cfg.enable_log_linecount = parse_bool(value);
 	else if (!strcmp(name, "enable-remote-branches"))
-		ctx.cfg.enable_remote_branches = atoi(value);
+		ctx.cfg.enable_remote_branches = parse_bool(value);
 	else if (!strcmp(name, "enable-subject-links"))
-		ctx.cfg.enable_subject_links = atoi(value);
+		ctx.cfg.enable_subject_links = parse_bool(value);
 	else if (!strcmp(name, "enable-html-serving"))
-		ctx.cfg.enable_html_serving = atoi(value);
+		ctx.cfg.enable_html_serving = parse_bool(value);
 	else if (!strcmp(name, "enable-tree-linenumbers"))
-		ctx.cfg.enable_tree_linenumbers = atoi(value);
+		ctx.cfg.enable_tree_linenumbers = parse_bool(value);
 	else if (!strcmp(name, "enable-git-config"))
-		ctx.cfg.enable_git_config = atoi(value);
+		ctx.cfg.enable_git_config = parse_bool(value);
 	else if (!strcmp(name, "max-stats"))
 		ctx.cfg.max_stats = cgit_find_stats_period(value, NULL);
 	else if (!strcmp(name, "cache-size"))
-		ctx.cfg.cache_size = atoi(value);
+		ctx.cfg.cache_size = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-root"))
 		ctx.cfg.cache_root = xstrdup(expand_macros(value));
 	else if (!strcmp(name, "cache-root-ttl"))
-		ctx.cfg.cache_root_ttl = atoi(value);
+		ctx.cfg.cache_root_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-repo-ttl"))
-		ctx.cfg.cache_repo_ttl = atoi(value);
+		ctx.cfg.cache_repo_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-scanrc-ttl"))
-		ctx.cfg.cache_scanrc_ttl = atoi(value);
+		ctx.cfg.cache_scanrc_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-static-ttl"))
-		ctx.cfg.cache_static_ttl = atoi(value);
+		ctx.cfg.cache_static_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-dynamic-ttl"))
-		ctx.cfg.cache_dynamic_ttl = atoi(value);
+		ctx.cfg.cache_dynamic_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-about-ttl"))
-		ctx.cfg.cache_about_ttl = atoi(value);
+		ctx.cfg.cache_about_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "cache-snapshot-ttl"))
-		ctx.cfg.cache_snapshot_ttl = atoi(value);
+		ctx.cfg.cache_snapshot_ttl = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "case-sensitive-sort"))
-		ctx.cfg.case_sensitive_sort = atoi(value);
+		ctx.cfg.case_sensitive_sort = parse_bool(value);
 	else if (!strcmp(name, "about-filter"))
 		ctx.cfg.about_filter = cgit_new_filter(value, ABOUT);
 	else if (!strcmp(name, "commit-filter"))
@@ -216,19 +231,19 @@ static void config_cb(const char *name, const char *value)
 	else if (!strcmp(name, "auth-filter"))
 		ctx.cfg.auth_filter = cgit_new_filter(value, AUTH);
 	else if (!strcmp(name, "embedded"))
-		ctx.cfg.embedded = atoi(value);
+		ctx.cfg.embedded = parse_bool(value);
 	else if (!strcmp(name, "max-atom-items"))
-		ctx.cfg.max_atom_items = atoi(value);
+		ctx.cfg.max_atom_items = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "max-message-length"))
-		ctx.cfg.max_msg_len = atoi(value);
+		ctx.cfg.max_msg_len = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "max-repodesc-length"))
-		ctx.cfg.max_repodesc_len = atoi(value);
+		ctx.cfg.max_repodesc_len = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "max-blob-size"))
-		ctx.cfg.max_blob_size = atoi(value);
+		ctx.cfg.max_blob_size = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "max-repo-count"))
-		ctx.cfg.max_repo_count = atoi(value);
+		ctx.cfg.max_repo_count = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "max-commit-count"))
-		ctx.cfg.max_commit_count = atoi(value);
+		ctx.cfg.max_commit_count = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "project-list"))
 		ctx.cfg.project_list = xstrdup(expand_macros(value));
 	else if (!strcmp(name, "scan-path"))
@@ -240,31 +255,31 @@ static void config_cb(const char *name, const char *value)
 		else
 			scan_tree(expand_macros(value), repo_config);
 	else if (!strcmp(name, "scan-hidden-path"))
-		ctx.cfg.scan_hidden_path = atoi(value);
+		ctx.cfg.scan_hidden_path = parse_bool(value);
 	else if (!strcmp(name, "section-from-path"))
-		ctx.cfg.section_from_path = atoi(value);
+		ctx.cfg.section_from_path = parse_bool(value);
 	else if (!strcmp(name, "repository-sort"))
 		ctx.cfg.repository_sort = xstrdup(value);
 	else if (!strcmp(name, "section-sort"))
-		ctx.cfg.section_sort = atoi(value);
+		ctx.cfg.section_sort = parse_bool(value);
 	else if (!strcmp(name, "source-filter"))
 		ctx.cfg.source_filter = cgit_new_filter(value, SOURCE);
 	else if (!strcmp(name, "summary-log"))
-		ctx.cfg.summary_log = atoi(value);
+		ctx.cfg.summary_log = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "summary-branches"))
-		ctx.cfg.summary_branches = atoi(value);
+		ctx.cfg.summary_branches = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "summary-tags"))
-		ctx.cfg.summary_tags = atoi(value);
+		ctx.cfg.summary_tags = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "side-by-side-diffs"))
-		ctx.cfg.difftype = atoi(value) ? DIFF_SSDIFF : DIFF_UNIFIED;
+		ctx.cfg.difftype = parse_bool(value) ? DIFF_SSDIFF : DIFF_UNIFIED;
 	else if (!strcmp(name, "agefile"))
 		ctx.cfg.agefile = xstrdup(value);
 	else if (!strcmp(name, "mimetype-file"))
 		ctx.cfg.mimetype_file = xstrdup(value);
 	else if (!strcmp(name, "renamelimit"))
-		ctx.cfg.renamelimit = atoi(value);
+		ctx.cfg.renamelimit = parse_int(value, -1, INT_MAX, 0);
 	else if (!strcmp(name, "remove-suffix"))
-		ctx.cfg.remove_suffix = atoi(value);
+		ctx.cfg.remove_suffix = parse_bool(value);
 	else if (!strcmp(name, "robots"))
 		ctx.cfg.robots = xstrdup(value);
 	else if (!strcmp(name, "clone-prefix"))
@@ -272,7 +287,7 @@ static void config_cb(const char *name, const char *value)
 	else if (!strcmp(name, "clone-url"))
 		ctx.cfg.clone_url = xstrdup(value);
 	else if (!strcmp(name, "local-time"))
-		ctx.cfg.local_time = atoi(value);
+		ctx.cfg.local_time = parse_bool(value);
 	else if (!strcmp(name, "commit-sort")) {
 		if (!strcmp(value, "date"))
 			ctx.cfg.commit_sort = 1;
@@ -318,7 +333,7 @@ static void querystring_cb(const char *name, const char *value)
 		ctx.qry.sha2 = xstrdup(value);
 		ctx.qry.has_sha1 = 1;
 	} else if (!strcmp(name, "ofs")) {
-		ctx.qry.ofs = atoi(value);
+		ctx.qry.ofs = parse_int(value, 0, INT_MAX, 0);
 	} else if (!strcmp(name, "path")) {
 		ctx.qry.path = trim_end(value, '/');
 	} else if (!strcmp(name, "name")) {
@@ -326,24 +341,24 @@ static void querystring_cb(const char *name, const char *value)
 	} else if (!strcmp(name, "s")) {
 		ctx.qry.sort = xstrdup(value);
 	} else if (!strcmp(name, "showmsg")) {
-		ctx.qry.showmsg = atoi(value);
+		ctx.qry.showmsg = parse_bool(value);
 	} else if (!strcmp(name, "period")) {
 		ctx.qry.period = xstrdup(value);
 	} else if (!strcmp(name, "dt")) {
-		ctx.qry.difftype = atoi(value);
+		ctx.qry.difftype = parse_bool(value);
 		ctx.qry.has_difftype = 1;
 	} else if (!strcmp(name, "ss")) {
 		/* No longer generated, but there may be links out there. */
-		ctx.qry.difftype = atoi(value) ? DIFF_SSDIFF : DIFF_UNIFIED;
+		ctx.qry.difftype = parse_bool(value) ? DIFF_SSDIFF : DIFF_UNIFIED;
 		ctx.qry.has_difftype = 1;
 	} else if (!strcmp(name, "all")) {
-		ctx.qry.show_all = atoi(value);
+		ctx.qry.show_all = parse_bool(value);
 	} else if (!strcmp(name, "context")) {
-		ctx.qry.context = atoi(value);
+		ctx.qry.context = parse_bool(value);
 	} else if (!strcmp(name, "ignorews")) {
-		ctx.qry.ignorews = atoi(value);
+		ctx.qry.ignorews = parse_bool(value);
 	} else if (!strcmp(name, "follow")) {
-		ctx.qry.follow = atoi(value);
+		ctx.qry.follow = parse_bool(value);
 	}
 }
 
@@ -987,7 +1002,7 @@ static void cgit_parse_args(int argc, const char **argv)
 			ctx.qry.sha1 = xstrdup(arg);
 			ctx.qry.has_sha1 = 1;
 		} else if (skip_prefix(argv[i], "--ofs=", &arg)) {
-			ctx.qry.ofs = atoi(arg);
+			ctx.qry.ofs = parse_int(arg, 0, INT_MAX, 0);
 		} else if (skip_prefix(argv[i], "--scan-tree=", &arg) ||
 		           skip_prefix(argv[i], "--scan-path=", &arg)) {
 			/*
diff --git a/cgit.h b/cgit.h
index bcc4fce..42e1429 100644
--- a/cgit.h
+++ b/cgit.h
@@ -26,6 +26,20 @@
 #include <notes.h>
 #include <graph.h>
 
+#ifndef MAX
+#define MAX(a,b) \
+   ({ __typeof__ (a) _a = (a); \
+      __typeof__ (b) _b = (b); \
+      _a > _b ? _a : _b; })
+#endif
+#ifndef MIN
+#define MIN(a,b) \
+   ({ __typeof__ (a) _a = (a); \
+      __typeof__ (b) _b = (b); \
+      _a < _b ? _a : _b; })
+#endif
+
+
 /* Add isgraph(x) to Git's sane ctype support (see git-compat-util.h) */
 #undef isgraph
 #define isgraph(x) (isprint((x)) && !isspace((x)))


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] cgit: sanitize max-repo-count
  2019-01-07 15:35           ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
@ 2019-01-07 15:35             ` list
  2019-01-07 20:54             ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
  2019-01-08 14:05             ` whydoubt
  2 siblings, 0 replies; 12+ messages in thread
From: list @ 2019-01-07 15:35 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

Setting max-repo-count to "0" makes cgit loop forever generating page
links. Make this a special value to show all repositories.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.c       | 2 +-
 cgitrc.5.txt | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/cgit.c b/cgit.c
index cc953a7..a09e3c7 100644
--- a/cgit.c
+++ b/cgit.c
@@ -241,7 +241,7 @@ static void config_cb(const char *name, const char *value)
 	else if (!strcmp(name, "max-blob-size"))
 		ctx.cfg.max_blob_size = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "max-repo-count"))
-		ctx.cfg.max_repo_count = parse_int(value, 0, INT_MAX, 0);
+		ctx.cfg.max_repo_count = parse_int(value, 0, INT_MAX, INT_MAX);
 	else if (!strcmp(name, "max-commit-count"))
 		ctx.cfg.max_commit_count = parse_int(value, 0, INT_MAX, 0);
 	else if (!strcmp(name, "project-list"))
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 34b351b..1731fda 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -269,7 +269,8 @@ max-message-length::
 
 max-repo-count::
 	Specifies the number of entries to list per page on the	repository
-	index page. Default value: "50".
+	index page. The value "0" shows all repositories without limitation.
+	Default value: "50".
 
 max-repodesc-length::
 	Specifies the maximum number of repo description characters to display


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing
  2019-01-07 15:35           ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
  2019-01-07 15:35             ` [PATCH 2/2] cgit: sanitize max-repo-count list
@ 2019-01-07 20:54             ` list
  2019-01-08 14:05             ` whydoubt
  2 siblings, 0 replies; 12+ messages in thread
From: list @ 2019-01-07 20:54 UTC (permalink / raw)


Christian Hesse <list at eworm.de> on Mon, 2019/01/07 16:35:
> From: Christian Hesse <mail at eworm.de>
> 
> We used to have atoi() only for parsing of numeric and boolean (numeric
> evaluating true or false) values. Let's introduce parse_{bool,int}()
> for minimal sanitization.

Please give this a good review to make sure I did not mess up. :-p
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20190107/43fb1cb0/attachment-0001.asc>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing
  2019-01-07 15:35           ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
  2019-01-07 15:35             ` [PATCH 2/2] cgit: sanitize max-repo-count list
  2019-01-07 20:54             ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
@ 2019-01-08 14:05             ` whydoubt
  2 siblings, 0 replies; 12+ messages in thread
From: whydoubt @ 2019-01-08 14:05 UTC (permalink / raw)


On Mon, Jan 7, 2019 at 9:36 AM Christian Hesse <list at eworm.de> wrote:

> From: Christian Hesse <mail at eworm.de>
>
> +       int value = MIN(MAX(atoi(str), max), min);
>

This will always result in value = min
The correct statement would be
  int value = MIN(MAX(atoi(str), min), max);

Alternately, since this is the only time you use MIN/MAX, you could instead
define a CLAMP macro and use it.
  int value = CLAMP(atoi(str), min, max);


> +               ctx.cfg.cache_size = parse_int(value, 0, INT_MAX, 0);
>

In all but a couple of instances, the same min/max/default values are
passed in.
Wouldn't it be better to have parse_int(value) for these, and something like
parse_int_clamp_default(value, min, max, default_if_zero) for the
exceptions?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20190108/4910b660/attachment.html>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] cgitrc: handle value "0" for max-repo-count
  2019-01-07 15:34         ` list
  2019-01-07 15:35           ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
@ 2019-01-10 19:23           ` tlatorre
  1 sibling, 0 replies; 12+ messages in thread
From: tlatorre @ 2019-01-10 19:23 UTC (permalink / raw)


Here's a function I've been using to safely convert strings to ints
with error codes:

int safe_strtoi(char *s, int *si)
{
    /* Convert string s -> integer. */
    char *end;
    long value;

    errno = 0;
    value = strtol(s, &end, 0);

    if (end == s) {
        return -1;
    } else if ('\0' != *end) {
        return -1;
    } else if (errno == ERANGE) {
        return -1;
    } else if (value > INT_MAX) {
        return -1;
    } else if (value < INT_MIN) {
        return -1;
    }

    *si = value;

    return 0;
}

On success it returns 0 and si is set to the integer value. On error,
it returns -1.

Tony

On Mon, Jan 7, 2019 at 9:35 AM Christian Hesse <list at eworm.de> wrote:
>
> Christian Hesse <list at eworm.de> on Fri, 2018/11/23 17:08:
> > "Jason A. Donenfeld" <Jason at zx2c4.com> on Fri, 2018/10/19 00:52:
> > > Hey Christian,
> > >
> > > We should indeed introduce some sanitation helpers to deal with these
> > > in the general case. API suggestion:
> > >
> > > type_t parse_int(const char *str, type_t min, type_t max, type_t
> > > fallback_if_invalid);
> > >
> > > What would you think of that?
> >
> > My intention was to add a special value 0. How about this?
> >
> > type_t parse_int(const char *str, type_t min, type_t max, type_t
> > default_if_zero, type_t fallback_if_invalid);
>
> As atoi() does not return error there is no "invalid"... Looks like we have
> to go with:
>
> type_t parse_int(const char *str, type_t min, type_t max, type_t
> default_if_zero);
> --
> main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
> "CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
> putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-01-10 19:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-16 14:38 [PATCH 1/1] cgitrc: handle value "0" for max-repo-count list
2018-07-16 14:41 ` list
2018-09-11  6:56   ` list
2018-10-18 22:52     ` Jason
2018-11-23 16:08       ` list
2018-12-07  4:10         ` Jason
2019-01-07 15:34         ` list
2019-01-07 15:35           ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
2019-01-07 15:35             ` [PATCH 2/2] cgit: sanitize max-repo-count list
2019-01-07 20:54             ` [PATCH 1/2] cgit: introduce parse_{bool, int}() for for cgitrc parsing list
2019-01-08 14:05             ` whydoubt
2019-01-10 19:23           ` [PATCH 1/1] cgitrc: handle value "0" for max-repo-count tlatorre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).