List for cgit developers and users
 help / color / mirror / Atom feed
* Custom snapshot prefix & Snapshot signatures
@ 2018-06-07 12:12 list
  2018-06-07 12:15 ` [PATCH 1/1] snapshot: support tar signature for compressed tar list
  2018-06-07 13:10 ` Custom snapshot prefix & Snapshot signatures john
  0 siblings, 2 replies; 18+ messages in thread
From: list @ 2018-06-07 12:12 UTC (permalink / raw)


Hello everybody,

I missed to reply in time and my mail client decided to purge old mails from
my mailing list folders. (Well, I told it to do so regularly.)
So breaking the threads and reviewing in this mail. Sorry!

First of all let me say that I really love this! The snapshot signatures
feature allows to get rid of uploading static tarballs and signatures,
handling everything from your command line with git. Thanks a lot for your
marvelous work, John!

I do agree with Konstantin, though: Binary compressed data may change over
time with more recent algorithms. So I will reply with an extra patch that
allows to push tar signatures to git notes that can be downloaded for
compressed tar snapshots.

Custom snapshot prefix [0]:
John Keeping (3):
  ui-shared: pass repo object to print_snapshot_links()
  ui-snapshot: pass repo into get_ref_from_filename()
  Add "snapshot-prefix" repo configuration

All these look good to me. Please add my Reviewed-by!

Snapshot signatures [1]:
John Keeping (7):
  ui-refs: remove unnecessary sanity check
  ui-shared: remove unused parameter
  ui-shared: rename parameter to cgit_print_snapshot_links()
  ui-shared: use the same snapshot logic as ui-refs
  ui-shared: pass separator in to cgit_print_snapshot_links()
  ui-refs: use shared function to print tag downloads

All these look good to me. Please add my Reviewed-by!

  snapshot: support archive signatures

> @@ -129,6 +154,39 @@ static int make_snapshot(const struct
> cgit_snapshot_format *format, return 0;
>  }
> 
> +static int write_sig(const struct cgit_snapshot_format *format,
> +		     const char *hex, const char *archive,
> +		     const char *filename)
> +{
> +	const struct object_id *note = cgit_snapshot_get_sig(hex, format);

Do we mix declaration and code? Let's get this separated.

> +	enum object_type type;
> +	unsigned long size;
> +	char *buf;
> [...]

But this is just nit-picking, so please add my Reviewed-by!

This will need some changes when updating to git v2.18.0. Would be nice to
have this in soon so I can rebase my commit.

[0] https://lists.zx2c4.com/pipermail/cgit/2018-March/003767.html
[1] https://lists.zx2c4.com/pipermail/cgit/2018-March/003772.html
-- 
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/20180607/a3339061/attachment.asc>


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

* [PATCH 1/1] snapshot: support tar signature for compressed tar
  2018-06-07 12:12 Custom snapshot prefix & Snapshot signatures list
@ 2018-06-07 12:15 ` list
  2018-06-07 13:17   ` john
  2018-06-07 13:10 ` Custom snapshot prefix & Snapshot signatures john
  1 sibling, 1 reply; 18+ messages in thread
From: list @ 2018-06-07 12:15 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

This adds support for kernel.org style signatures where the uncompressed
tar archive is signed and compressed later. The signature is valid for
all tar* snapshots.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c   | 8 ++++++++
 ui-snapshot.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ui-shared.c b/ui-shared.c
index 8a786e0..40935ae 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1139,6 +1139,14 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
 					   filename.buf);
 			html(")");
+		} else if (f->bit & 0x16 && cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[3])) {
+			int suf_len = strlen(f->suffix);
+			strbuf_remove(&filename, strlen(filename.buf) - suf_len, suf_len);
+			strbuf_addstr(&filename, ".tar.asc");
+			html(" (");
+			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
+					   filename.buf);
+			html(")");
 		}
 		html(separator);
 	}
diff --git a/ui-snapshot.c b/ui-snapshot.c
index c7611e8..76d0573 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -263,7 +263,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
 	}
 
 	f = get_format(filename);
-	if (!f || !(ctx.repo->snapshots & f->bit)) {
+	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
 		cgit_print_error_page(400, "Bad request",
 				"Unsupported snapshot format: %s", filename);
 		return;


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

* Custom snapshot prefix & Snapshot signatures
  2018-06-07 12:12 Custom snapshot prefix & Snapshot signatures list
  2018-06-07 12:15 ` [PATCH 1/1] snapshot: support tar signature for compressed tar list
@ 2018-06-07 13:10 ` john
  1 sibling, 0 replies; 18+ messages in thread
From: john @ 2018-06-07 13:10 UTC (permalink / raw)


On Thu, Jun 07, 2018 at 02:12:51PM +0200, Christian Hesse wrote:
> Hello everybody,
> 
> I missed to reply in time and my mail client decided to purge old mails from
> my mailing list folders. (Well, I told it to do so regularly.)
> So breaking the threads and reviewing in this mail. Sorry!
> 
> First of all let me say that I really love this! The snapshot signatures
> feature allows to get rid of uploading static tarballs and signatures,
> handling everything from your command line with git. Thanks a lot for your
> marvelous work, John!
> 
> I do agree with Konstantin, though: Binary compressed data may change over
> time with more recent algorithms. So I will reply with an extra patch that
> allows to push tar signatures to git notes that can be downloaded for
> compressed tar snapshots.
> 
> Custom snapshot prefix [0]:
> John Keeping (3):
>   ui-shared: pass repo object to print_snapshot_links()
>   ui-snapshot: pass repo into get_ref_from_filename()
>   Add "snapshot-prefix" repo configuration
> 
> All these look good to me. Please add my Reviewed-by!
> 
> Snapshot signatures [1]:
> John Keeping (7):
>   ui-refs: remove unnecessary sanity check
>   ui-shared: remove unused parameter
>   ui-shared: rename parameter to cgit_print_snapshot_links()
>   ui-shared: use the same snapshot logic as ui-refs
>   ui-shared: pass separator in to cgit_print_snapshot_links()
>   ui-refs: use shared function to print tag downloads
> 
> All these look good to me. Please add my Reviewed-by!
> 
>   snapshot: support archive signatures
> 
> > @@ -129,6 +154,39 @@ static int make_snapshot(const struct
> > cgit_snapshot_format *format, return 0;
> >  }
> > 
> > +static int write_sig(const struct cgit_snapshot_format *format,
> > +		     const char *hex, const char *archive,
> > +		     const char *filename)
> > +{
> > +	const struct object_id *note = cgit_snapshot_get_sig(hex, format);
> 
> Do we mix declaration and code? Let's get this separated.

I think this is fine, it isn't decl-after-statement which is what we
object to.  Initialize-at-declaration is common in git.git and in CGit.

> > +	enum object_type type;
> > +	unsigned long size;
> > +	char *buf;
> > [...]
> 
> But this is just nit-picking, so please add my Reviewed-by!
> 
> This will need some changes when updating to git v2.18.0. Would be nice to
> have this in soon so I can rebase my commit.
> 
> [0] https://lists.zx2c4.com/pipermail/cgit/2018-March/003767.html
> [1] https://lists.zx2c4.com/pipermail/cgit/2018-March/003772.html


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

* [PATCH 1/1] snapshot: support tar signature for compressed tar
  2018-06-07 12:15 ` [PATCH 1/1] snapshot: support tar signature for compressed tar list
@ 2018-06-07 13:17   ` john
  2018-06-07 15:13     ` list
  0 siblings, 1 reply; 18+ messages in thread
From: john @ 2018-06-07 13:17 UTC (permalink / raw)


On Thu, Jun 07, 2018 at 02:15:34PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> This adds support for kernel.org style signatures where the uncompressed
> tar archive is signed and compressed later. The signature is valid for
> all tar* snapshots.
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  ui-shared.c   | 8 ++++++++
>  ui-snapshot.c | 2 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/ui-shared.c b/ui-shared.c
> index 8a786e0..40935ae 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -1139,6 +1139,14 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
>  			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
>  					   filename.buf);
>  			html(")");
> +		} else if (f->bit & 0x16 && cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[3])) {

This works, but it feels far too magic and likely to break in the
future.  I'd rather add a new field for base snapshot type, either as a
const char * set that to ".tar" for the relevant archive formats or as a
bitmask which is set to 0x08 for now to allow fallback to tar.  If we do
that, we should extract at least that bit value to a named constant to
make it clear what is going on.

> +			int suf_len = strlen(f->suffix);
> +			strbuf_remove(&filename, strlen(filename.buf) - suf_len, suf_len);
> +			strbuf_addstr(&filename, ".tar.asc");
> +			html(" (");
> +			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> +					   filename.buf);
> +			html(")");
>  		}
>  		html(separator);
>  	}
> diff --git a/ui-snapshot.c b/ui-snapshot.c
> index c7611e8..76d0573 100644
> --- a/ui-snapshot.c
> +++ b/ui-snapshot.c
> @@ -263,7 +263,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
>  	}
>  
>  	f = get_format(filename);
> -	if (!f || !(ctx.repo->snapshots & f->bit)) {
> +	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {

This bypasses the permitted snapshots configuration, but I guess that's
ok because signature lookup is cheap unlike archive creation.

>  		cgit_print_error_page(400, "Bad request",
>  				"Unsupported snapshot format: %s", filename);
>  		return;


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

* [PATCH 1/1] snapshot: support tar signature for compressed tar
  2018-06-07 13:17   ` john
@ 2018-06-07 15:13     ` list
  2018-06-07 15:14       ` [PATCH v2 1/2] ui-snapshot: use named constants for snapshot formats list
  0 siblings, 1 reply; 18+ messages in thread
From: list @ 2018-06-07 15:13 UTC (permalink / raw)


John Keeping <john at keeping.me.uk> on Thu, 2018/06/07 14:17:
> On Thu, Jun 07, 2018 at 02:15:34PM +0200, Christian Hesse wrote:
> > From: Christian Hesse <mail at eworm.de>
> > 
> > This adds support for kernel.org style signatures where the uncompressed
> > tar archive is signed and compressed later. The signature is valid for
> > all tar* snapshots.
> > 
> > Signed-off-by: Christian Hesse <mail at eworm.de>
> > ---
> >  ui-shared.c   | 8 ++++++++
> >  ui-snapshot.c | 2 +-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ui-shared.c b/ui-shared.c
> > index 8a786e0..40935ae 100644
> > --- a/ui-shared.c
> > +++ b/ui-shared.c
> > @@ -1139,6 +1139,14 @@ void cgit_print_snapshot_links(const struct
> > cgit_repo *repo, const char *ref, cgit_snapshot_link("sig", NULL, NULL,
> > NULL, NULL, filename.buf);
> >  			html(")");
> > +		} else if (f->bit & 0x16 && cgit_snapshot_get_sig(ref,
> > &cgit_snapshot_formats[3])) {  
> 
> This works, but it feels far too magic and likely to break in the
> future.  I'd rather add a new field for base snapshot type, either as a
> const char * set that to ".tar" for the relevant archive formats or as a
> bitmask which is set to 0x08 for now to allow fallback to tar.  If we do
> that, we should extract at least that bit value to a named constant to
> make it clear what is going on.

Already working on that. ;)

What concerns me a lot more is the fact that we rely on the array position.
If anybody decides to insert a new element in cgit_snapshot_formats before
the tar element things break.

I decided to reorder the element and add a comment about what we rely on...
Wondering whether or not you like it and if there are any better ideas. :-p
I prefer this order anyway.

> > +			int suf_len = strlen(f->suffix);
> > +			strbuf_remove(&filename, strlen(filename.buf) -
> > suf_len, suf_len);
> > +			strbuf_addstr(&filename, ".tar.asc");
> > +			html(" (");
> > +			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> > +					   filename.buf);
> > +			html(")");
> >  		}
> >  		html(separator);
> >  	}
> > diff --git a/ui-snapshot.c b/ui-snapshot.c
> > index c7611e8..76d0573 100644
> > --- a/ui-snapshot.c
> > +++ b/ui-snapshot.c
> > @@ -263,7 +263,7 @@ void cgit_print_snapshot(const char *head, const char
> > *hex, }
> >  
> >  	f = get_format(filename);
> > -	if (!f || !(ctx.repo->snapshots & f->bit)) {
> > +	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {  
> 
> This bypasses the permitted snapshots configuration, but I guess that's
> ok because signature lookup is cheap unlike archive creation.

It does, but for signatures only. I think it is not worth the trouble making
the condition even more complex to deny ".zip.asc".

> >  		cgit_print_error_page(400, "Bad request",
> >  				"Unsupported snapshot format: %s",
> > filename); return;  



-- 
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/20180607/81e256ab/attachment.asc>


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

* [PATCH v2 1/2] ui-snapshot: use named constants for snapshot formats
  2018-06-07 15:13     ` list
@ 2018-06-07 15:14       ` list
  2018-06-07 15:14         ` [PATCH v2 2/2] snapshot: support tar signature for compressed tar list
  0 siblings, 1 reply; 18+ messages in thread
From: list @ 2018-06-07 15:14 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

Instead of using bitmasks directly we use named constants for snapshot
formats. This makes thing more comprehensible.

We also change the element order and move tar to the top. Note this
change is visible in UI for the user!

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.h        | 10 ++++++++++
 ui-snapshot.c | 16 +++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/cgit.h b/cgit.h
index a686390..02d63ca 100644
--- a/cgit.h
+++ b/cgit.h
@@ -310,10 +310,20 @@ struct cgit_context {
 
 typedef int (*write_archive_fn_t)(const char *, const char *);
 
+enum cgit_snapshot_type {
+	CGIT_SNAPSHOT_NONE	= 0x00,
+	CGIT_SNAPSHOT_TAR	= 0x01,
+	CGIT_SNAPSHOT_TAR_GZ	= 0x02,
+	CGIT_SNAPSHOT_TAR_BZ2	= 0x04,
+	CGIT_SNAPSHOT_TAR_XZ	= 0x08,
+	CGIT_SNAPSHOT_ZIP	= 0x10
+};
+
 struct cgit_snapshot_format {
 	const char *suffix;
 	const char *mimetype;
 	write_archive_fn_t write_func;
+	int base;
 	int bit;
 };
 
diff --git a/ui-snapshot.c b/ui-snapshot.c
index c7611e8..c9ec1f3 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -86,11 +86,17 @@ static int write_tar_xz_archive(const char *hex, const char *prefix)
 }
 
 const struct cgit_snapshot_format cgit_snapshot_formats[] = {
-	{ ".zip", "application/x-zip", write_zip_archive, 0x01 },
-	{ ".tar.gz", "application/x-gzip", write_tar_gzip_archive, 0x02 },
-	{ ".tar.bz2", "application/x-bzip2", write_tar_bzip2_archive, 0x04 },
-	{ ".tar", "application/x-tar", write_tar_archive, 0x08 },
-	{ ".tar.xz", "application/x-xz", write_tar_xz_archive, 0x10 },
+	/* Keep tar the first! */
+	{ ".tar",	"application/x-tar",	write_tar_archive,
+		CGIT_SNAPSHOT_NONE,	CGIT_SNAPSHOT_TAR	},
+	{ ".tar.gz",	"application/x-gzip",	write_tar_gzip_archive,
+		CGIT_SNAPSHOT_TAR,	CGIT_SNAPSHOT_TAR_GZ	},
+	{ ".tar.bz2",	"application/x-bzip2",	write_tar_bzip2_archive,
+		CGIT_SNAPSHOT_TAR,	CGIT_SNAPSHOT_TAR_BZ2	},
+	{ ".tar.xz",	"application/x-xz",	write_tar_xz_archive,
+		CGIT_SNAPSHOT_TAR,	CGIT_SNAPSHOT_TAR_XZ	},
+	{ ".zip",	"application/x-zip",	write_zip_archive,
+		CGIT_SNAPSHOT_NONE,	CGIT_SNAPSHOT_ZIP	},
 	{ NULL }
 };
 


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

* [PATCH v2 2/2] snapshot: support tar signature for compressed tar
  2018-06-07 15:14       ` [PATCH v2 1/2] ui-snapshot: use named constants for snapshot formats list
@ 2018-06-07 15:14         ` list
  2018-06-07 15:21           ` john
  0 siblings, 1 reply; 18+ messages in thread
From: list @ 2018-06-07 15:14 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

This adds support for kernel.org style signatures where the uncompressed
tar archive is signed and compressed later. The signature is valid for
all tar* snapshots.

We have a filter which snapshots may be generated and downloaded. This has
to allow tar signatures now even if tar itself is not allowed. To simplify
things we allow all signatures.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c   | 8 ++++++++
 ui-snapshot.c | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index 8a786e0..72c0a33 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1139,6 +1139,14 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
 					   filename.buf);
 			html(")");
+		} else if (f->base == CGIT_SNAPSHOT_TAR && cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[0])) {
+			int suf_len = strlen(f->suffix);
+			strbuf_remove(&filename, strlen(filename.buf) - suf_len, suf_len);
+			strbuf_addstr(&filename, ".tar.asc");
+			html(" (");
+			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
+					   filename.buf);
+			html(")");
 		}
 		html(separator);
 	}
diff --git a/ui-snapshot.c b/ui-snapshot.c
index c9ec1f3..07a6447 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -86,7 +86,7 @@ static int write_tar_xz_archive(const char *hex, const char *prefix)
 }
 
 const struct cgit_snapshot_format cgit_snapshot_formats[] = {
-	/* Keep tar the first! */
+	/* Keep tar the first! Signature download relies on this. */
 	{ ".tar",	"application/x-tar",	write_tar_archive,
 		CGIT_SNAPSHOT_NONE,	CGIT_SNAPSHOT_TAR	},
 	{ ".tar.gz",	"application/x-gzip",	write_tar_gzip_archive,
@@ -269,7 +269,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
 	}
 
 	f = get_format(filename);
-	if (!f || !(ctx.repo->snapshots & f->bit)) {
+	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
 		cgit_print_error_page(400, "Bad request",
 				"Unsupported snapshot format: %s", filename);
 		return;


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

* [PATCH v2 2/2] snapshot: support tar signature for compressed tar
  2018-06-07 15:14         ` [PATCH v2 2/2] snapshot: support tar signature for compressed tar list
@ 2018-06-07 15:21           ` john
  2018-06-07 19:36             ` list
  2018-06-08 22:11             ` [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format list
  0 siblings, 2 replies; 18+ messages in thread
From: john @ 2018-06-07 15:21 UTC (permalink / raw)


On Thu, Jun 07, 2018 at 05:14:52PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> This adds support for kernel.org style signatures where the uncompressed
> tar archive is signed and compressed later. The signature is valid for
> all tar* snapshots.
> 
> We have a filter which snapshots may be generated and downloaded. This has
> to allow tar signatures now even if tar itself is not allowed. To simplify
> things we allow all signatures.
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  ui-shared.c   | 8 ++++++++
>  ui-snapshot.c | 4 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/ui-shared.c b/ui-shared.c
> index 8a786e0..72c0a33 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -1139,6 +1139,14 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
>  			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
>  					   filename.buf);
>  			html(")");
> +		} else if (f->base == CGIT_SNAPSHOT_TAR && cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[0])) {

I was thinking we could just to the lookup here, so walk the array to
find the base signature type.  That avoids the dependency on the order
of the table.

What I'd really like to do is avoid storing the "bit" field in the array
and just calculate it from the index, something like:

	1u << (fmt - &cgit_snapshot_formats[0])

but I haven't followed all of the consumers of the snapshots array to
figure out if it's easy to convert them.

> +			int suf_len = strlen(f->suffix);
> +			strbuf_remove(&filename, strlen(filename.buf) - suf_len, suf_len);
> +			strbuf_addstr(&filename, ".tar.asc");
> +			html(" (");
> +			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> +					   filename.buf);
> +			html(")");
>  		}
>  		html(separator);
>  	}
> diff --git a/ui-snapshot.c b/ui-snapshot.c
> index c9ec1f3..07a6447 100644
> --- a/ui-snapshot.c
> +++ b/ui-snapshot.c
> @@ -86,7 +86,7 @@ static int write_tar_xz_archive(const char *hex, const char *prefix)
>  }
>  
>  const struct cgit_snapshot_format cgit_snapshot_formats[] = {
> -	/* Keep tar the first! */
> +	/* Keep tar the first! Signature download relies on this. */
>  	{ ".tar",	"application/x-tar",	write_tar_archive,
>  		CGIT_SNAPSHOT_NONE,	CGIT_SNAPSHOT_TAR	},
>  	{ ".tar.gz",	"application/x-gzip",	write_tar_gzip_archive,
> @@ -269,7 +269,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
>  	}
>  
>  	f = get_format(filename);
> -	if (!f || !(ctx.repo->snapshots & f->bit)) {
> +	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
>  		cgit_print_error_page(400, "Bad request",
>  				"Unsupported snapshot format: %s", filename);
>  		return;


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

* [PATCH v2 2/2] snapshot: support tar signature for compressed tar
  2018-06-07 15:21           ` john
@ 2018-06-07 19:36             ` list
  2018-06-07 19:38               ` [PATCH v3 1/1] " list
  2018-06-08 22:11             ` [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format list
  1 sibling, 1 reply; 18+ messages in thread
From: list @ 2018-06-07 19:36 UTC (permalink / raw)


John Keeping <john at keeping.me.uk> on Thu, 2018/06/07 16:21:
> I was thinking we could just to the lookup here, so walk the array to
> find the base signature type.  That avoids the dependency on the order
> of the table.

Did that, works perfectly. I will reply with patch v3.

> What I'd really like to do is avoid storing the "bit" field in the array
> and just calculate it from the index, something like:
> 
> 	1u << (fmt - &cgit_snapshot_formats[0])
> 
> but I haven't followed all of the consumers of the snapshots array to
> figure out if it's easy to convert them.

Let's have a look at that later, currently I do not need it.
-- 
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/20180607/c10b1702/attachment.asc>


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

* [PATCH v3 1/1] snapshot: support tar signature for compressed tar
  2018-06-07 19:36             ` list
@ 2018-06-07 19:38               ` list
  2018-06-27 16:34                 ` Jason
  0 siblings, 1 reply; 18+ messages in thread
From: list @ 2018-06-07 19:38 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

This adds support for kernel.org style signatures where the uncompressed
tar archive is signed and compressed later. The signature is valid for
all tar* snapshots.

We have a filter which snapshots may be generated and downloaded. This has
to allow tar signatures now even if tar itself is not allowed. To simplify
things we allow all signatures.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c   | 12 +++++++++++-
 ui-snapshot.c |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index 8a786e0..51a25a0 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1114,7 +1114,7 @@ void cgit_compose_snapshot_prefix(struct strbuf *filename, const char *base,
 void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 			       const char *separator)
 {
-	const struct cgit_snapshot_format* f;
+	const struct cgit_snapshot_format *f, *f_tar;
 	struct strbuf filename = STRBUF_INIT;
 	const char *basename;
 	size_t prefixlen;
@@ -1125,6 +1125,9 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 	else
 		cgit_compose_snapshot_prefix(&filename, basename, ref);
 
+	for (f_tar = cgit_snapshot_formats; strcmp(f_tar->suffix, ".tar") != 0; f_tar++)
+		/* nothing */ ;
+
 	prefixlen = filename.len;
 	for (f = cgit_snapshot_formats; f->suffix; f++) {
 		if (!(repo->snapshots & f->bit))
@@ -1139,6 +1142,13 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
 					   filename.buf);
 			html(")");
+		} else if (starts_with(f->suffix, ".tar") && cgit_snapshot_get_sig(ref, f_tar)) {
+			strbuf_setlen(&filename, strlen(filename.buf) - strlen(f->suffix));
+			strbuf_addstr(&filename, ".tar.asc");
+			html(" (");
+			cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
+					   filename.buf);
+			html(")");
 		}
 		html(separator);
 	}
diff --git a/ui-snapshot.c b/ui-snapshot.c
index c7611e8..76d0573 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -263,7 +263,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
 	}
 
 	f = get_format(filename);
-	if (!f || !(ctx.repo->snapshots & f->bit)) {
+	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
 		cgit_print_error_page(400, "Bad request",
 				"Unsupported snapshot format: %s", filename);
 		return;


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

* [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format
  2018-06-07 15:21           ` john
  2018-06-07 19:36             ` list
@ 2018-06-08 22:11             ` list
  2018-06-09 11:16               ` john
  1 sibling, 1 reply; 18+ messages in thread
From: list @ 2018-06-08 22:11 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

We had a static bit value in struct cgit_snapshot_format. We do not rely
on it and things can be calculated on the fly. So strip it.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.c        |  3 ++-
 cgit.h        |  1 -
 shared.c      |  5 ++++-
 ui-shared.c   |  3 ++-
 ui-snapshot.c | 28 ++++++++++------------------
 5 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/cgit.c b/cgit.c
index d2f7b9c..5f634fc 100644
--- a/cgit.c
+++ b/cgit.c
@@ -763,9 +763,10 @@ static char *build_snapshot_setting(int bitmap)
 {
 	const struct cgit_snapshot_format *f;
 	struct strbuf result = STRBUF_INIT;
+	int i = 0;
 
 	for (f = cgit_snapshot_formats; f->suffix; f++) {
-		if (f->bit & bitmap) {
+		if (1 << i++ & bitmap) {
 			if (result.len)
 				strbuf_addch(&result, ' ');
 			strbuf_addstr(&result, f->suffix);
diff --git a/cgit.h b/cgit.h
index a686390..707a3f5 100644
--- a/cgit.h
+++ b/cgit.h
@@ -314,7 +314,6 @@ struct cgit_snapshot_format {
 	const char *suffix;
 	const char *mimetype;
 	write_archive_fn_t write_func;
-	int bit;
 };
 
 extern const char *cgit_version;
diff --git a/shared.c b/shared.c
index 0a11e68..c61a0a1 100644
--- a/shared.c
+++ b/shared.c
@@ -397,12 +397,15 @@ int cgit_parse_snapshots_mask(const char *str)
 	string_list_remove_empty_items(&tokens, 0);
 
 	for_each_string_list_item(item, &tokens) {
+		int i = 0;
+
 		for (f = cgit_snapshot_formats; f->suffix; f++) {
 			if (!strcmp(item->string, f->suffix) ||
 			    !strcmp(item->string, f->suffix + 1)) {
-				rv |= f->bit;
+				rv |= 1 << i;
 				break;
 			}
+			i++;
 		}
 	}
 
diff --git a/ui-shared.c b/ui-shared.c
index 51a25a0..6092004 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1118,6 +1118,7 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 	struct strbuf filename = STRBUF_INIT;
 	const char *basename;
 	size_t prefixlen;
+	int i = 0;
 
 	basename = cgit_snapshot_prefix(repo);
 	if (starts_with(ref, basename))
@@ -1130,7 +1131,7 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 
 	prefixlen = filename.len;
 	for (f = cgit_snapshot_formats; f->suffix; f++) {
-		if (!(repo->snapshots & f->bit))
+		if (!(repo->snapshots & 1 << i++))
 			continue;
 		strbuf_setlen(&filename, prefixlen);
 		strbuf_addstr(&filename, f->suffix);
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 76d0573..04f41cc 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -86,11 +86,11 @@ static int write_tar_xz_archive(const char *hex, const char *prefix)
 }
 
 const struct cgit_snapshot_format cgit_snapshot_formats[] = {
-	{ ".zip", "application/x-zip", write_zip_archive, 0x01 },
-	{ ".tar.gz", "application/x-gzip", write_tar_gzip_archive, 0x02 },
-	{ ".tar.bz2", "application/x-bzip2", write_tar_bzip2_archive, 0x04 },
-	{ ".tar", "application/x-tar", write_tar_archive, 0x08 },
-	{ ".tar.xz", "application/x-xz", write_tar_xz_archive, 0x10 },
+	{ ".tar",	"application/x-tar",	write_tar_archive	},
+	{ ".tar.gz",	"application/x-gzip",	write_tar_gzip_archive	},
+	{ ".tar.bz2",	"application/x-bzip2",	write_tar_bzip2_archive	},
+	{ ".tar.xz",	"application/x-xz",	write_tar_xz_archive	},
+	{ ".zip",	"application/x-zip",	write_zip_archive	},
 	{ NULL }
 };
 
@@ -119,17 +119,6 @@ const struct object_id *cgit_snapshot_get_sig(const char *ref,
 	return get_note(tree, &oid);
 }
 
-static const struct cgit_snapshot_format *get_format(const char *filename)
-{
-	const struct cgit_snapshot_format *fmt;
-
-	for (fmt = cgit_snapshot_formats; fmt->suffix; fmt++) {
-		if (ends_with(filename, fmt->suffix))
-			return fmt;
-	}
-	return NULL;
-}
-
 static int make_snapshot(const struct cgit_snapshot_format *format,
 			 const char *hex, const char *prefix,
 			 const char *filename)
@@ -246,6 +235,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
 	const char *sig_filename = NULL;
 	char *adj_filename = NULL;
 	char *prefix = NULL;
+	int i = 0;
 
 	if (!filename) {
 		cgit_print_error_page(400, "Bad request",
@@ -262,8 +252,10 @@ void cgit_print_snapshot(const char *head, const char *hex,
 		filename = adj_filename;
 	}
 
-	f = get_format(filename);
-	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
+	for (f = cgit_snapshot_formats; f->suffix && !ends_with(filename, f->suffix); f++)
+		i++;
+
+	if (!f->suffix || (!sig_filename && !(ctx.repo->snapshots & 1 << i))) {
 		cgit_print_error_page(400, "Bad request",
 				"Unsupported snapshot format: %s", filename);
 		return;


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

* [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format
  2018-06-08 22:11             ` [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format list
@ 2018-06-09 11:16               ` john
  2018-06-11  6:51                 ` list
  0 siblings, 1 reply; 18+ messages in thread
From: john @ 2018-06-09 11:16 UTC (permalink / raw)


On Sat, Jun 09, 2018 at 12:11:11AM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> We had a static bit value in struct cgit_snapshot_format. We do not rely
> on it and things can be calculated on the fly. So strip it.
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  cgit.c        |  3 ++-
>  cgit.h        |  1 -
>  shared.c      |  5 ++++-
>  ui-shared.c   |  3 ++-
>  ui-snapshot.c | 28 ++++++++++------------------
>  5 files changed, 18 insertions(+), 22 deletions(-)
> 
> diff --git a/cgit.c b/cgit.c
> index d2f7b9c..5f634fc 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -763,9 +763,10 @@ static char *build_snapshot_setting(int bitmap)
>  {
>  	const struct cgit_snapshot_format *f;
>  	struct strbuf result = STRBUF_INIT;
> +	int i = 0;
>  
>  	for (f = cgit_snapshot_formats; f->suffix; f++) {
> -		if (f->bit & bitmap) {
> +		if (1 << i++ & bitmap) {

I think this would be nicer if we introduce a BIT macro like Linux has,
something like:

	#define BIT(x)	(1U << (x))

but even better, can't we have something like:

	static inline unsigned cgit_snapshot_bit(
		const struct cgit_snapshot_format *f)
	{
		return BIT(f - &cgit_snapshot_formats[0]);
	}

and avoid needing the loop index completely?

>  			if (result.len)
>  				strbuf_addch(&result, ' ');
>  			strbuf_addstr(&result, f->suffix);
> diff --git a/cgit.h b/cgit.h
> index a686390..707a3f5 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -314,7 +314,6 @@ struct cgit_snapshot_format {
>  	const char *suffix;
>  	const char *mimetype;
>  	write_archive_fn_t write_func;
> -	int bit;
>  };
>  
>  extern const char *cgit_version;
> diff --git a/shared.c b/shared.c
> index 0a11e68..c61a0a1 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -397,12 +397,15 @@ int cgit_parse_snapshots_mask(const char *str)
>  	string_list_remove_empty_items(&tokens, 0);
>  
>  	for_each_string_list_item(item, &tokens) {
> +		int i = 0;
> +
>  		for (f = cgit_snapshot_formats; f->suffix; f++) {
>  			if (!strcmp(item->string, f->suffix) ||
>  			    !strcmp(item->string, f->suffix + 1)) {
> -				rv |= f->bit;
> +				rv |= 1 << i;
>  				break;
>  			}
> +			i++;
>  		}
>  	}
>  
> diff --git a/ui-shared.c b/ui-shared.c
> index 51a25a0..6092004 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -1118,6 +1118,7 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
>  	struct strbuf filename = STRBUF_INIT;
>  	const char *basename;
>  	size_t prefixlen;
> +	int i = 0;
>  
>  	basename = cgit_snapshot_prefix(repo);
>  	if (starts_with(ref, basename))
> @@ -1130,7 +1131,7 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
>  
>  	prefixlen = filename.len;
>  	for (f = cgit_snapshot_formats; f->suffix; f++) {
> -		if (!(repo->snapshots & f->bit))
> +		if (!(repo->snapshots & 1 << i++))
>  			continue;
>  		strbuf_setlen(&filename, prefixlen);
>  		strbuf_addstr(&filename, f->suffix);
> diff --git a/ui-snapshot.c b/ui-snapshot.c
> index 76d0573..04f41cc 100644
> --- a/ui-snapshot.c
> +++ b/ui-snapshot.c
> @@ -86,11 +86,11 @@ static int write_tar_xz_archive(const char *hex, const char *prefix)
>  }
>  
>  const struct cgit_snapshot_format cgit_snapshot_formats[] = {
> -	{ ".zip", "application/x-zip", write_zip_archive, 0x01 },
> -	{ ".tar.gz", "application/x-gzip", write_tar_gzip_archive, 0x02 },
> -	{ ".tar.bz2", "application/x-bzip2", write_tar_bzip2_archive, 0x04 },
> -	{ ".tar", "application/x-tar", write_tar_archive, 0x08 },
> -	{ ".tar.xz", "application/x-xz", write_tar_xz_archive, 0x10 },
> +	{ ".tar",	"application/x-tar",	write_tar_archive	},
> +	{ ".tar.gz",	"application/x-gzip",	write_tar_gzip_archive	},
> +	{ ".tar.bz2",	"application/x-bzip2",	write_tar_bzip2_archive	},
> +	{ ".tar.xz",	"application/x-xz",	write_tar_xz_archive	},
> +	{ ".zip",	"application/x-zip",	write_zip_archive	},
>  	{ NULL }
>  };
>  
> @@ -119,17 +119,6 @@ const struct object_id *cgit_snapshot_get_sig(const char *ref,
>  	return get_note(tree, &oid);
>  }
>  
> -static const struct cgit_snapshot_format *get_format(const char *filename)
> -{
> -	const struct cgit_snapshot_format *fmt;
> -
> -	for (fmt = cgit_snapshot_formats; fmt->suffix; fmt++) {
> -		if (ends_with(filename, fmt->suffix))
> -			return fmt;
> -	}
> -	return NULL;
> -}
> -
>  static int make_snapshot(const struct cgit_snapshot_format *format,
>  			 const char *hex, const char *prefix,
>  			 const char *filename)
> @@ -246,6 +235,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
>  	const char *sig_filename = NULL;
>  	char *adj_filename = NULL;
>  	char *prefix = NULL;
> +	int i = 0;
>  
>  	if (!filename) {
>  		cgit_print_error_page(400, "Bad request",
> @@ -262,8 +252,10 @@ void cgit_print_snapshot(const char *head, const char *hex,
>  		filename = adj_filename;
>  	}
>  
> -	f = get_format(filename);
> -	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
> +	for (f = cgit_snapshot_formats; f->suffix && !ends_with(filename, f->suffix); f++)
> +		i++;
> +
> +	if (!f->suffix || (!sig_filename && !(ctx.repo->snapshots & 1 << i))) {
>  		cgit_print_error_page(400, "Bad request",
>  				"Unsupported snapshot format: %s", filename);
>  		return;
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit


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

* [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format
  2018-06-09 11:16               ` john
@ 2018-06-11  6:51                 ` list
  2018-06-11  6:53                   ` [PATCH v2 " list
  0 siblings, 1 reply; 18+ messages in thread
From: list @ 2018-06-11  6:51 UTC (permalink / raw)


John Keeping <john at keeping.me.uk> on Sat, 2018/06/09 12:16:
> I think this would be nicer if we introduce a BIT macro like Linux has,
> something like:
> 
> 	#define BIT(x)	(1U << (x))

Reasonable. ;)

> but even better, can't we have something like:
> 
> 	static inline unsigned cgit_snapshot_bit(
> 		const struct cgit_snapshot_format *f)
> 	{
> 		return BIT(f - &cgit_snapshot_formats[0]);
> 	}
> 
> and avoid needing the loop index completely?

Ok, finally got the idea. :-p
I will reply with an updated patch.
-- 
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/20180611/068d49fe/attachment.asc>


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

* [PATCH v2 1/1] snapshot: strip bit from struct cgit_snapshot_format
  2018-06-11  6:51                 ` list
@ 2018-06-11  6:53                   ` list
  0 siblings, 0 replies; 18+ messages in thread
From: list @ 2018-06-11  6:53 UTC (permalink / raw)


From: Christian Hesse <mail at eworm.de>

We had a static bit value in struct cgit_snapshot_format. We do not rely
on it and things can be calculated on the fly. So strip it.

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.c        |  2 +-
 cgit.h        |  4 +++-
 shared.c      |  2 +-
 ui-shared.c   |  2 +-
 ui-snapshot.c | 17 +++++++++++------
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/cgit.c b/cgit.c
index d2f7b9c..ca0a89c 100644
--- a/cgit.c
+++ b/cgit.c
@@ -765,7 +765,7 @@ static char *build_snapshot_setting(int bitmap)
 	struct strbuf result = STRBUF_INIT;
 
 	for (f = cgit_snapshot_formats; f->suffix; f++) {
-		if (f->bit & bitmap) {
+		if (cgit_snapshot_format_bit(f) & bitmap) {
 			if (result.len)
 				strbuf_addch(&result, ' ');
 			strbuf_addstr(&result, f->suffix);
diff --git a/cgit.h b/cgit.h
index a686390..0798dc5 100644
--- a/cgit.h
+++ b/cgit.h
@@ -46,6 +46,8 @@
  */
 #define PAGE_ENCODING "UTF-8"
 
+#define BIT(x)	(1U << (x))
+
 typedef void (*configfn)(const char *name, const char *value);
 typedef void (*filepair_fn)(struct diff_filepair *pair);
 typedef void (*linediff_fn)(char *line, int len);
@@ -314,7 +316,6 @@ struct cgit_snapshot_format {
 	const char *suffix;
 	const char *mimetype;
 	write_archive_fn_t write_func;
-	int bit;
 };
 
 extern const char *cgit_version;
@@ -376,6 +377,7 @@ extern const char *cgit_repobasename(const char *reponame);
 extern int cgit_parse_snapshots_mask(const char *str);
 extern const struct object_id *cgit_snapshot_get_sig(const char *ref,
 						     const struct cgit_snapshot_format *f);
+extern const unsigned cgit_snapshot_format_bit(const struct cgit_snapshot_format *f);
 
 extern int cgit_open_filter(struct cgit_filter *filter, ...);
 extern int cgit_close_filter(struct cgit_filter *filter);
diff --git a/shared.c b/shared.c
index 0a11e68..d59ae7e 100644
--- a/shared.c
+++ b/shared.c
@@ -400,7 +400,7 @@ int cgit_parse_snapshots_mask(const char *str)
 		for (f = cgit_snapshot_formats; f->suffix; f++) {
 			if (!strcmp(item->string, f->suffix) ||
 			    !strcmp(item->string, f->suffix + 1)) {
-				rv |= f->bit;
+				rv |= cgit_snapshot_format_bit(f);
 				break;
 			}
 		}
diff --git a/ui-shared.c b/ui-shared.c
index 51a25a0..b773a9c 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1130,7 +1130,7 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 
 	prefixlen = filename.len;
 	for (f = cgit_snapshot_formats; f->suffix; f++) {
-		if (!(repo->snapshots & f->bit))
+		if (!(repo->snapshots & cgit_snapshot_format_bit(f)))
 			continue;
 		strbuf_setlen(&filename, prefixlen);
 		strbuf_addstr(&filename, f->suffix);
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 76d0573..4ebd295 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -86,11 +86,11 @@ static int write_tar_xz_archive(const char *hex, const char *prefix)
 }
 
 const struct cgit_snapshot_format cgit_snapshot_formats[] = {
-	{ ".zip", "application/x-zip", write_zip_archive, 0x01 },
-	{ ".tar.gz", "application/x-gzip", write_tar_gzip_archive, 0x02 },
-	{ ".tar.bz2", "application/x-bzip2", write_tar_bzip2_archive, 0x04 },
-	{ ".tar", "application/x-tar", write_tar_archive, 0x08 },
-	{ ".tar.xz", "application/x-xz", write_tar_xz_archive, 0x10 },
+	{ ".tar",	"application/x-tar",	write_tar_archive	},
+	{ ".tar.gz",	"application/x-gzip",	write_tar_gzip_archive	},
+	{ ".tar.bz2",	"application/x-bzip2",	write_tar_bzip2_archive	},
+	{ ".tar.xz",	"application/x-xz",	write_tar_xz_archive	},
+	{ ".zip",	"application/x-zip",	write_zip_archive	},
 	{ NULL }
 };
 
@@ -130,6 +130,11 @@ static const struct cgit_snapshot_format *get_format(const char *filename)
 	return NULL;
 }
 
+const unsigned cgit_snapshot_format_bit(const struct cgit_snapshot_format *f)
+{
+	return BIT(f - &cgit_snapshot_formats[0]);
+}
+
 static int make_snapshot(const struct cgit_snapshot_format *format,
 			 const char *hex, const char *prefix,
 			 const char *filename)
@@ -263,7 +268,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
 	}
 
 	f = get_format(filename);
-	if (!f || (!sig_filename && !(ctx.repo->snapshots & f->bit))) {
+	if (!f || (!sig_filename && !(ctx.repo->snapshots & cgit_snapshot_format_bit(f)))) {
 		cgit_print_error_page(400, "Bad request",
 				"Unsupported snapshot format: %s", filename);
 		return;


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

* [PATCH v3 1/1] snapshot: support tar signature for compressed tar
  2018-06-07 19:38               ` [PATCH v3 1/1] " list
@ 2018-06-27 16:34                 ` Jason
  2018-06-27 20:14                   ` john
  0 siblings, 1 reply; 18+ messages in thread
From: Jason @ 2018-06-27 16:34 UTC (permalink / raw)


Hey Christian,

I've merged all the surrounding changes, but I'm not quite satisfied
with the implementation of this one.

> +       for (f_tar = cgit_snapshot_formats; strcmp(f_tar->suffix, ".tar") != 0; f_tar++)
> +               /* nothing */ ;
> +
> +               } else if (starts_with(f->suffix, ".tar") && cgit_snapshot_get_sig(ref, f_tar)) {
> +                       strbuf_setlen(&filename, strlen(filename.buf) - strlen(f->suffix));
> +                       strbuf_addstr(&filename, ".tar.asc");
> +                       html(" (");
> +                       cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> +                                          filename.buf);
> +                       html(")");

Can we, instead, _not_ special case .tar, but rather just allow for
all signatures, if the note .asc exists? We don't want to serve
arbitrary tarballs and archives, because this means load and bandwidth
for the server that wasn't explicitly opted in by the admin, but all
signatures are necessarily explicitly uploaded, so why restrict them
from being downloaded?

Regards,
Jason


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

* [PATCH v3 1/1] snapshot: support tar signature for compressed tar
  2018-06-27 16:34                 ` Jason
@ 2018-06-27 20:14                   ` john
  2018-07-02  7:10                     ` list
  0 siblings, 1 reply; 18+ messages in thread
From: john @ 2018-06-27 20:14 UTC (permalink / raw)


On Wed, Jun 27, 2018 at 06:34:56PM +0200, Jason A. Donenfeld wrote:
> I've merged all the surrounding changes, but I'm not quite satisfied
> with the implementation of this one.
> 
> > +       for (f_tar = cgit_snapshot_formats; strcmp(f_tar->suffix, ".tar") != 0; f_tar++)
> > +               /* nothing */ ;
> > +
> > +               } else if (starts_with(f->suffix, ".tar") && cgit_snapshot_get_sig(ref, f_tar)) {
> > +                       strbuf_setlen(&filename, strlen(filename.buf) - strlen(f->suffix));
> > +                       strbuf_addstr(&filename, ".tar.asc");
> > +                       html(" (");
> > +                       cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> > +                                          filename.buf);
> > +                       html(")");
> 
> Can we, instead, _not_ special case .tar, but rather just allow for
> all signatures, if the note .asc exists? We don't want to serve
> arbitrary tarballs and archives, because this means load and bandwidth
> for the server that wasn't explicitly opted in by the admin, but all
> signatures are necessarily explicitly uploaded, so why restrict them
> from being downloaded?

I'm not quite sure what you're asking here, this is just printing the
signature link after the snapshow download link.

The idea here is that if you are downloading a .tar.gz then the
signature for the base .tar is better (it's easier to consistently
generate a .tar than it is a .tar.gz), so the admin will choose to
provide .tar.asc instead of .tar.gz.asc.

I would quite like to avoid special-casing .tar in the code like this
and instead allow a fallback option (or even bitmask) in the formats
table as a more generic implementation, but I don't think that's your
complaint here (I also don't think we'll ever add it for other formats,
so hardcoding .tar isn't too bad).


John


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

* [PATCH v3 1/1] snapshot: support tar signature for compressed tar
  2018-06-27 20:14                   ` john
@ 2018-07-02  7:10                     ` list
  2018-07-03 18:56                       ` Jason
  0 siblings, 1 reply; 18+ messages in thread
From: list @ 2018-07-02  7:10 UTC (permalink / raw)


John Keeping <john at keeping.me.uk> on Wed, 2018/06/27 21:14:
> On Wed, Jun 27, 2018 at 06:34:56PM +0200, Jason A. Donenfeld wrote:
> > I've merged all the surrounding changes, but I'm not quite satisfied
> > with the implementation of this one.
> >   
> > > +       for (f_tar = cgit_snapshot_formats; strcmp(f_tar->suffix,
> > > ".tar") != 0; f_tar++)
> > > +               /* nothing */ ;
> > > +
> > > +               } else if (starts_with(f->suffix, ".tar") &&
> > > cgit_snapshot_get_sig(ref, f_tar)) {
> > > +                       strbuf_setlen(&filename, strlen(filename.buf) -
> > > strlen(f->suffix));
> > > +                       strbuf_addstr(&filename, ".tar.asc");
> > > +                       html(" (");
> > > +                       cgit_snapshot_link("sig", NULL, NULL, NULL,
> > > NULL,
> > > +                                          filename.buf);
> > > +                       html(")");  
> > 
> > Can we, instead, _not_ special case .tar, but rather just allow for
> > all signatures, if the note .asc exists? We don't want to serve
> > arbitrary tarballs and archives, because this means load and bandwidth
> > for the server that wasn't explicitly opted in by the admin, but all
> > signatures are necessarily explicitly uploaded, so why restrict them
> > from being downloaded?  
> 
> I'm not quite sure what you're asking here, this is just printing the
> signature link after the snapshow download link.
> 
> The idea here is that if you are downloading a .tar.gz then the
> signature for the base .tar is better (it's easier to consistently
> generate a .tar than it is a .tar.gz), so the admin will choose to
> provide .tar.asc instead of .tar.gz.asc.

John is right. Actually we do allow all signatures to be downloaded, but
choose where to show the tar signature downloads. Providing .tar.asc for .zip
ist pointless, no? :-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/20180702/c4c6caf8/attachment.asc>


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

* [PATCH v3 1/1] snapshot: support tar signature for compressed tar
  2018-07-02  7:10                     ` list
@ 2018-07-03 18:56                       ` Jason
  0 siblings, 0 replies; 18+ messages in thread
From: Jason @ 2018-07-03 18:56 UTC (permalink / raw)


I've merged a variant of the patch, and it appears to be working well.
Thanks guys!


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

end of thread, other threads:[~2018-07-03 18:56 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 12:12 Custom snapshot prefix & Snapshot signatures list
2018-06-07 12:15 ` [PATCH 1/1] snapshot: support tar signature for compressed tar list
2018-06-07 13:17   ` john
2018-06-07 15:13     ` list
2018-06-07 15:14       ` [PATCH v2 1/2] ui-snapshot: use named constants for snapshot formats list
2018-06-07 15:14         ` [PATCH v2 2/2] snapshot: support tar signature for compressed tar list
2018-06-07 15:21           ` john
2018-06-07 19:36             ` list
2018-06-07 19:38               ` [PATCH v3 1/1] " list
2018-06-27 16:34                 ` Jason
2018-06-27 20:14                   ` john
2018-07-02  7:10                     ` list
2018-07-03 18:56                       ` Jason
2018-06-08 22:11             ` [PATCH 1/1] snapshot: strip bit from struct cgit_snapshot_format list
2018-06-09 11:16               ` john
2018-06-11  6:51                 ` list
2018-06-11  6:53                   ` [PATCH v2 " list
2018-06-07 13:10 ` Custom snapshot prefix & Snapshot signatures john

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).