[Patch] Add a function to append to an existing capture #247

Closed
guyharris opened this Issue Apr 16, 2013 · 11 comments

Projects

None yet

4 participants

@guyharris
Member

Converted from SourceForge issue 3086711, submitted by mark-jn

This patch adds a function that can be used to append to an existing capture file. If the file doesn't exist or is empty, the function will simply write a new header; otherwise, it reads the existing header in and verifies that the new link layer type is the same as that in the capture file.

Sponsored by Sandvine Incorporated.

Thanks,
-Mark

@infrastation
Member

The suggested patch:

--- /usr/src/contrib/libpcap/pcap/pcap.h        2009-04-11 13:36:11.000000000 -0400
+++ pcap/pcap.h 2010-10-13 10:53:16.000000000 -0400
@@ -322,6 +322,7 @@ int pcap_fileno(pcap_t *);

 pcap_dumper_t *pcap_dump_open(pcap_t *, const char *);
 pcap_dumper_t *pcap_dump_fopen(pcap_t *, FILE *fp);
+pcap_dumper_t *pcap_dump_append(pcap_t *, const char *);
 FILE   *pcap_dump_file(pcap_dumper_t *);
 long   pcap_dump_ftell(pcap_dumper_t *);
 int    pcap_dump_flush(pcap_dumper_t *);
--- /usr/src/contrib/libpcap/savefile.c 2009-03-24 11:57:35.000000000 -0400
+++ savefile.c  2010-10-13 10:53:09.000000000 -0400
@@ -42,6 +42,7 @@ static const char rcsid[] _U_ =
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>

 #include "pcap-int.h"
 #include "pcap/usb.h"
@@ -1737,6 +1738,64 @@ pcap_dump_flush(pcap_dumper_t *p)
                return (0);
 }

+pcap_dumper_t *
+pcap_dump_append(pcap_t *p, const char *fname)
+{
+       FILE *f;
+       int linktype;
+       int exists = 0, amt_read;
+       struct pcap_file_header ph;
+
+       linktype = dlt_to_linktype(p->linktype);
+       if (linktype == -1) {
+               snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+                   "%s: link-layer type %d isn't supported in savefiles",
+                   fname, linktype);
+               return (NULL);
+       }
+       if (fname[0] == '-' && fname[1] == '\0')
+               f = stdout;
+
+       else {
+               exists = !access(fname, R_OK);
+               f = fopen(fname, "r+");
+               if (f == NULL) {
+                       snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "%s: %s",
+                           fname, pcap_strerror(errno));
+                       return (NULL);
+               }
+
+               /* Read the header and make sure it's of the same linktype. */
+               amt_read = fread(&ph, 1, sizeof (ph), f);
+
+               if (amt_read != sizeof (ph)) {
+                       if (ferror(f)) {
+                               snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "%s: %s",
+                                   fname, pcap_strerror(errno));
+                               return (NULL);
+                       } else if (feof(f) && amt_read > 0) {
+                               snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+                                   "%s: truncated pcap file header", fname);
+                               return (NULL);
+                       }
+               }
+
+               /* If a header is already present and doesn't match
+                * the linktype, return an error. */
+               if (amt_read > 0 && linktype != ph.linktype) {
+                       snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+                           "%s: invalid linktype, cannot append to file",
+                           fname);
+                       return (NULL);
+               }
+
+               lseek(fileno(f), 0, SEEK_END);
+       }
+       if (!exists)
+               (void)sf_write_header(f, linktype, p->tzoff, p->snapshot);
+       return ((pcap_dumper_t *)f);
+}
+
 void
 pcap_dump_close(pcap_dumper_t *p)
 {
@guyharris
Member

This should probably also make sure that the snapshot lengths are the same, and fail if they're not.

@guyharris
Member

Most of the comments in this mail thread seemed to be positive or neutral; does anybody want to argue against this functionality being provided at all? If not, are there any complaints about the name pcap_dump_append()? Perhaps pcap_dump_open_append() to make it clearer that this call doesn't append a packet, it just opens an output file for appending?

@NinnOgTonic

I guess we will just wait another 4 years then.

@infrastation
Member

pcap_dump_open_append() looks more clear. Please note the patch above is from 2010, the patch on the mailing list thread is from 2011 and is a bit different, a copy of it is below. In the 2011 revision three fclose() calls seem to be missing, one before each of the return NULL lines. The author email address (for git commit --author) is available from the mailing list thread.

diff --git a/pcap/pcap.h b/pcap/pcap.h
index 05ba31f..abf5d5b 100644
--- a/pcap/pcap.h
+++ b/pcap/pcap.h
@@ -337,6 +337,7 @@ int pcap_fileno(pcap_t *);

 pcap_dumper_t *pcap_dump_open(pcap_t *, const char *);
 pcap_dumper_t *pcap_dump_fopen(pcap_t *, FILE *fp);
+pcap_dumper_t *pcap_dump_append(pcap_t *, const char *);
 FILE   *pcap_dump_file(pcap_dumper_t *);
 long   pcap_dump_ftell(pcap_dumper_t *);
 int    pcap_dump_flush(pcap_dumper_t *);
diff --git a/sf-pcap.c b/sf-pcap.c
index 9d55dae..a3b0757 100644
--- a/sf-pcap.c
+++ b/sf-pcap.c
@@ -56,6 +56,7 @@ static const char rcsid[] _U_ =
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>

 #include "pcap-int.h"

@@ -579,6 +580,65 @@ pcap_dump_fopen(pcap_t *p, FILE *f)
        return (pcap_setup_dump(p, linktype, f, "stream"));
 }

+pcap_dumper_t *
+pcap_dump_append(pcap_t *p, const char *fname)
+{
+
+       FILE *f;
+       int linktype;
+       int exists = 0, amt_read;
+       struct pcap_file_header ph;
+
+       linktype = dlt_to_linktype(p->linktype);
+       if (linktype == -1) {
+               snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+                   "%s: link-layer type %d isn't supported in savefiles",
+                   fname, linktype);
+               return (NULL);
+       }
+       if (fname[0] == '-' && fname[1] == '\0') {
+               sf_write_header(stdout, linktype, p->tzoff, p->snapshot);
+               return ((pcap_dumper_t *)stdout);
+       }
+
+       exists = !access(fname, R_OK);
+       f = fopen(fname, "r+");
+       if (f == NULL) {
+               snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "%s: %s",
+                   fname, pcap_strerror(errno));
+               return (NULL);
+       }
+
+       /* Read the header and make sure it's of the same linktype. */
+       amt_read = fread(&ph, 1, sizeof (ph), f);
+       if (amt_read != sizeof (ph)) {
+               if (ferror(f)) {
+                       snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "%s: %s",
+                           fname, pcap_strerror(errno));
+                       return (NULL);
+               } else if (feof(f) && amt_read > 0) {
+                       snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+                           "%s: truncated pcap file header", fname);
+                       return (NULL);
+               }
+       }
+
+       /*
+        * If a header is already present and doesn't match the linktype,
+        * return an error.
+        */
+       if (amt_read > 0 && linktype != ph.linktype) {
+               snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+                   "%s: invalid linktype, cannot append to file", fname);
+               return (NULL);
+       }
+
+       fseek(f, 0, SEEK_END);
+       if (!exists)
+               (void)sf_write_header(f, linktype, p->tzoff, p->snapshot);
+       return ((pcap_dumper_t *)f);
+}
+
 FILE *
 pcap_dump_file(pcap_dumper_t *p)
 {
@guyharris guyharris added a commit that referenced this issue Feb 16, 2015
@guyharris guyharris Add pcap_dump_open_append() to open for appending.
See GitHub issue #247.
3cd7422
@guyharris
Member

Fixed in 3cd7422; the version checked in has a number of additional checks added, and has a man page update as well.

@guyharris guyharris closed this Feb 16, 2015
@guyharris
Member

The current version will fail if the file doesn't exist, because it opens with "r+". Should it instead create the file if it doesn't exist, opening with "w+"?

@marmeladema

I've also patched my version of libpcap to use mode a+/ab+ to create the file if it does not exists AND append at the end of file.
I don't think w+/wb+ would work because the file would be truncated.

Should this issue be reopened ?

@guyharris
Member

Only if "append" is to be interpreted as "...and create a new file if it doesn't exist" rather than "append to an existing file".

Note that the "create a new file if it doesn't exist" code path has to be different from the "append to an existing file" code path - the first code path needs to write a file header - so just changing "r+" to "a+" will not work.

@marmeladema

The code already takes care if this case (line 902 in sf-pcap.c):

	} else {
		
		/*
		 * A header isn't present; attempt to write it.
		 */
		if (sf_write_header(p, f, linktype, p->tzoff, p->snapshot) == -1) {
			pcap_snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "Can't write to %s: %s",
			    fname, pcap_strerror(errno));
			(void)fclose(f);
			return (NULL);
		}
	}

I just changed r to a and it works from what i can tell.

@marmeladema

Only if "append" is to be interpreted as "...and create a new file if it doesn't exist" rather than "append to an existing file".

Currently "open" already stand for "..and create a new file if it doesn't exist", so "open_append" could mean "open and create a new file if doesn't exist and append to it if it does"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment