Bug#674060: Doesn't support reading InRelease files

2012-05-31 Thread Mehdi Dogguy 

On 31/05/12 12:17, Bastian Blank wrote:

On Fri, May 25, 2012 at 11:01:57AM +0200, Mehdi Dogguy wrote:

index 67aa412..0b0f802 100644
--- a/src/parser_rfc822.c
+++ b/src/parser_rfc822.c
@@ -60,6 +60,7 @@ int di_parser_rfc822_read (char *begin, size_t size, 
di_parser_info *info, di_pa
di_rstring field_modifier_string;
di_rstring value_string;
void *act = NULL;
+  int pgp_mode = 0;

cur = begin;
end = begin + size;
@@ -81,6 +82,25 @@ int di_parser_rfc822_read (char *begin, size_t size, 
di_parser_info *info, di_pa

  while (1)
  {
+  if (!strncmp(cur, "-BEGIN PGP SIGNED MESSAGE-", 34))


Please don't use magic numbers.
| const char pgp_begin = "…";
| if (!strncmp(cur, pgp_begin, strlen(pgp_begin))
or something like that. The strlen call should be optimized away.

Also this string may only appear at the beginning of the file, not
somewhere in between. So this should be done before the large loop.
Also the end of the preamble can be searched with \n\n.


+  else if (pgp_mode&&  !strncmp(cur, "-BEGIN PGP SIGNATURE-", 29))
+  {
+// Let's exit, the rest of the file is not interesting
+cur += size;
+break;
+  }


Okay.



Thanks for the review! Please find attached an updated patch taking
into consideration your remarks.

Cheers,

--
Mehdi Dogguy مهدي الدڤي
http://dogguy.org/
>From c0b6669243e2282e0010d74790373764fe77b5ab Mon Sep 17 00:00:00 2001
From: Mehdi Dogguy 
Date: Wed, 23 May 2012 13:18:59 +0200
Subject: [PATCH] Add support for InRelease files (Closes: #674060)

---
 src/parser_rfc822.c |   18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/parser_rfc822.c b/src/parser_rfc822.c
index 67aa412..eca63c6 100644
--- a/src/parser_rfc822.c
+++ b/src/parser_rfc822.c
@@ -60,6 +60,9 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   di_rstring field_modifier_string;
   di_rstring value_string;
   void *act = NULL;
+  int pgp_mode = 0;
+  const char *pgp_begin_msg = "-BEGIN PGP SIGNED MESSAGE-";
+  const char *pgp_begin_sig = "-BEGIN PGP SIGNATURE-";
 
   cur = begin;
   end = begin + size;
@@ -72,6 +75,15 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   continue;
 }
 
+if (!strncmp(cur, pgp_begin_msg, strlen(pgp_begin_msg)))
+{
+  // Enable pgp_mode
+  pgp_mode = 1;
+  // Skip PGP header
+  cur = strstr (cur, "\n\n");
+  cur += 2;
+}
+
 nr++;
 
 if (entry_new)
@@ -81,6 +93,12 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
 
 while (1)
 {
+  if (pgp_mode && !strncmp(cur, pgp_begin_sig, strlen(pgp_begin_sig)))
+  {
+// Let's exit, the rest of the file is not interesting
+cur += size;
+break;
+  }
   field_begin = cur;
   readsize = end - field_begin < READSIZE ? end - field_begin : READSIZE;
   if (!readsize)
-- 
1.7.10

Bug#674060: Doesn't support reading InRelease files

2012-05-31 Thread Bastian Blank 
On Fri, May 25, 2012 at 11:01:57AM +0200, Mehdi Dogguy wrote:
> index 67aa412..0b0f802 100644
> --- a/src/parser_rfc822.c
> +++ b/src/parser_rfc822.c
> @@ -60,6 +60,7 @@ int di_parser_rfc822_read (char *begin, size_t size, 
> di_parser_info *info, di_pa
>di_rstring field_modifier_string;
>di_rstring value_string;
>void *act = NULL;
> +  int pgp_mode = 0;
>  
>cur = begin;
>end = begin + size;
> @@ -81,6 +82,25 @@ int di_parser_rfc822_read (char *begin, size_t size, 
> di_parser_info *info, di_pa
>  
>  while (1)
>  {
> +  if (!strncmp(cur, "-BEGIN PGP SIGNED MESSAGE-", 34))

Please don't use magic numbers.
| const char pgp_begin = "…";
| if (!strncmp(cur, pgp_begin, strlen(pgp_begin))
or something like that. The strlen call should be optimized away.

Also this string may only appear at the beginning of the file, not
somewhere in between. So this should be done before the large loop.
Also the end of the preamble can be searched with \n\n.

> +  else if (pgp_mode && !strncmp(cur, "-BEGIN PGP SIGNATURE-", 
> 29))
> +  {
> +// Let's exit, the rest of the file is not interesting
> +cur += size;
> +break;
> +  }

Okay.

Bastian

-- 
The best diplomat I know is a fully activated phaser bank.
-- Scotty



--
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20120531101729.ga2...@wavehammer.waldi.eu.org

Bug#674060: Doesn't support reading InRelease files

2012-05-25 Thread Mehdi Dogguy 

On 24/05/12 17:57, Mehdi Dogguy wrote:


On 22/05/12 21:36, Mehdi Dogguy wrote:


It seems that src/release.c:di_release_read_file can't read
InRelease files (yet) because it is not strictly an rfc822 file.



Please find attached a tentative patch to add support for InRelease
files to libd-i. It makes src/parser_rfc822.c:di_parser_rfc822_read
skip PGP signature and headers.



My first patch had a minor issue: PGP clearsign signatures may contain
one or more "Hash" fields. The PGP signature header and the signed data
are separated by an empty line (not included in the signed data). So it
seems more reasonable (if pgp_mode) to skip entries until a empty line
is reached. This is fixed in the new patch attached to this message. For
your convenience, I also attach "changes.diff" which gives the
difference between the two patches.

There is one missing check (to be fully RFC 2440 compliant), the data
needs to be dash-unescaped. While this could be mandatory in general, it
seems unnecessary in out specific case (where we read Release, Packages,
Sources, Contents and Translation files) as it means that the read file
doesn't follow its spec.

Comments are welcome.

Regards,

--
Mehdi Dogguy مهدي الدڤي
http://dogguy.org/
--- a/src/parser_rfc822.c
+++ b/src/parser_rfc822.c
@@ -89,12 +89,18 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
 // Let's skip this line
 cur += 35;
   }
-  else if (!strncmp(cur, "-BEGIN PGP SIGNATURE-", 29))
+  else if (pgp_mode && !strncmp(cur, "-BEGIN PGP SIGNATURE-", 29))
   {
 // Let's exit, the rest of the file is not interesting
 cur += size;
 break;
   }
+  if (pgp_mode == 1 && *cur == '\n') {
+// Skip this empty line (last line of PGP headers)
+cur++;
+// Do not skip more newlines
+pgp_mode++;
+  }
   field_begin = cur;
   readsize = end - field_begin < READSIZE ? end - field_begin : READSIZE;
   if (!readsize)
@@ -159,11 +165,9 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   }
   value_size = value_end - value_begin;
 
-  if (pgp_mode == 1 && !strncmp(field_begin, "Hash", field_size)) {
-// Do not skip more "Hash" fields
-pgp_mode++;
-// Skip this entry (4 == ':' + ' ' + '\n' + '\n')
-cur += field_size + value_size + 4;
+  if (pgp_mode == 1) {
+// Skip this entry (3 == ':' + ' ' + '\n')
+cur += field_size + value_size + 3;
 continue;
   }
 
>From 415831b3aa2a18019c1777698b449c0a8f2aca05 Mon Sep 17 00:00:00 2001
From: Mehdi Dogguy 
Date: Wed, 23 May 2012 13:18:59 +0200
Subject: [PATCH] Add support for InRelease files (Closes: #674060)

---
 src/parser_rfc822.c |   26 ++
 1 file changed, 26 insertions(+)

diff --git a/src/parser_rfc822.c b/src/parser_rfc822.c
index 67aa412..0b0f802 100644
--- a/src/parser_rfc822.c
+++ b/src/parser_rfc822.c
@@ -60,6 +60,7 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   di_rstring field_modifier_string;
   di_rstring value_string;
   void *act = NULL;
+  int pgp_mode = 0;
 
   cur = begin;
   end = begin + size;
@@ -81,6 +82,25 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
 
 while (1)
 {
+  if (!strncmp(cur, "-BEGIN PGP SIGNED MESSAGE-", 34))
+  {
+// Enable pgp_mode
+pgp_mode = 1;
+// Let's skip this line
+cur += 35;
+  }
+  else if (pgp_mode && !strncmp(cur, "-BEGIN PGP SIGNATURE-", 29))
+  {
+// Let's exit, the rest of the file is not interesting
+cur += size;
+break;
+  }
+  if (pgp_mode == 1 && *cur == '\n') {
+// Skip this empty line (last line of PGP headers)
+cur++;
+// Do not skip more newlines
+pgp_mode++;
+  }
   field_begin = cur;
   readsize = end - field_begin < READSIZE ? end - field_begin : READSIZE;
   if (!readsize)
@@ -145,6 +165,12 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   }
   value_size = value_end - value_begin;
 
+  if (pgp_mode == 1) {
+// Skip this entry (3 == ':' + ' ' + '\n')
+cur += field_size + value_size + 3;
+continue;
+  }
+
   field_string.string = field_begin;
   field_string.size = field_size;
   value_string.string = value_begin;
-- 
1.7.10

Processed: Re: Bug#674060: Doesn't support reading InRelease files

2012-05-24 Thread Debian Bug Tracking System 
Processing commands for cont...@bugs.debian.org:

> tags 674060 + patch
Bug #674060 [src:libdebian-installer] Doesn't support reading InRelease files
Added tag(s) patch.
> thanks
Stopping processing here.

Please contact me if you need assistance.
-- 
674060: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=674060
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems


--
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/handler.s.c.133787512512872.transcr...@bugs.debian.org

Bug#674060: Doesn't support reading InRelease files

2012-05-24 Thread Mehdi Dogguy 

tags 674060 + patch
thanks

On 22/05/12 21:36, Mehdi Dogguy wrote:


It seems that src/release.c:di_release_read_file can't read
InRelease files (yet) because it is not strictly an rfc822 file.



Please find attached a tentative patch to add support for InRelease
files to libd-i. It makes src/parser_rfc822.c:di_parser_rfc822_read skip
PGP signature and headers.

Regards,

--
Mehdi Dogguy مهدي الدڤي
http://dogguy.org/
>From caeda32359b1f3cbff3da9777d646f92f8c6b479 Mon Sep 17 00:00:00 2001
From: Mehdi Dogguy 
Date: Wed, 23 May 2012 13:18:59 +0200
Subject: [PATCH] Add support for InRelease files (Closes: #674060)

---
 src/parser_rfc822.c |   22 ++
 1 file changed, 22 insertions(+)

diff --git a/src/parser_rfc822.c b/src/parser_rfc822.c
index 67aa412..7049223 100644
--- a/src/parser_rfc822.c
+++ b/src/parser_rfc822.c
@@ -60,6 +60,7 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   di_rstring field_modifier_string;
   di_rstring value_string;
   void *act = NULL;
+  int pgp_mode = 0;
 
   cur = begin;
   end = begin + size;
@@ -81,6 +82,19 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
 
 while (1)
 {
+  if (!strncmp(cur, "-BEGIN PGP SIGNED MESSAGE-", 34))
+  {
+// Enable pgp_mode
+pgp_mode = 1;
+// Let's skip this line
+cur += 35;
+  }
+  else if (!strncmp(cur, "-BEGIN PGP SIGNATURE-", 29))
+  {
+// Let's exit, the rest of the file is not interesting
+cur += size;
+break;
+  }
   field_begin = cur;
   readsize = end - field_begin < READSIZE ? end - field_begin : READSIZE;
   if (!readsize)
@@ -145,6 +159,14 @@ int di_parser_rfc822_read (char *begin, size_t size, di_parser_info *info, di_pa
   }
   value_size = value_end - value_begin;
 
+  if (pgp_mode == 1 && !strncmp(field_begin, "Hash", field_size)) {
+// Do not skip more "Hash" fields
+pgp_mode++;
+// Skip this entry (4 == ':' + ' ' + '\n' + '\n')
+cur += field_size + value_size + 4;
+continue;
+  }
+
   field_string.string = field_begin;
   field_string.size = field_size;
   value_string.string = value_begin;
-- 
1.7.10

Bug#674060: Doesn't support reading InRelease files

2012-05-22 Thread Mehdi Dogguy 
Source: libdebian-installer
Version: 0.80
Severity: important
Tags: d-i

Hi,

It seems that src/release.c:di_release_read_file can't read InRelease
files (yet) because it is not strictly an rfc822 file.

It would be nice to fix this in time for Wheezy so that we can
consider getting rid of Release{,.gpg} files after Wheezy release.

This issue blocks #673625.

Regards,

-- System Information:
Debian Release: wheezy/sid
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) (ignored: LC_ALL 
set to en_GB.UTF-8)
Shell: /bin/sh linked to /bin/dash



-- 
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20120522193627.12051.79029.reportbug@potassium