[PATCH wayland 3/3] Validate the protocol xml during scanning

[Pekka Paalanen]

On Mon,  9 Nov 2015 14:15:01 +1000
Peter Hutterer <peter.hutterer at who-t.net> wrote:

> Embed the wayland.dtd protocol data into the scanner binary so we can validate
> external protocol files without requiring makefile changes. Hat-tip to Pekka
> Paalanen for the embedding trick.
> The embedding trick doesn't work well if the to-be-embedded file is in a
> different location than the source file, so copy/link it during configure and
> then build it in from the local directory.
> 
> The current expat parser is not a validating parser, moving scanner.c to
> another parser has the risk of breaking compatibility. This patch adds libxml2
> as extra (optional) dependency, but that also requires parsing the input
> twice.
> 
> If the protocol fails validation a warning is printed but no error is returned
> otherwise.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  Makefile.am   |  9 ++++++--
>  configure.ac  | 12 ++++++++++
>  src/dtddata.S | 40 ++++++++++++++++++++++++++++++++++
>  src/scanner.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 129 insertions(+), 2 deletions(-)
>  create mode 100644 src/dtddata.S

> diff --git a/Makefile.am b/Makefile.am
> index 9114d98..e2e52ed 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -24,10 +24,15 @@ pkgconfig_DATA =
>  
>  bin_PROGRAMS = wayland-scanner
>  wayland_scanner_SOURCES = src/scanner.c
> -wayland_scanner_CFLAGS = $(EXPAT_CFLAGS) $(AM_CFLAGS)
> -wayland_scanner_LDADD = $(EXPAT_LIBS) libwayland-util.la
> +wayland_scanner_CFLAGS = $(EXPAT_CFLAGS) $(LIBXML_CFLAGS) $(AM_CFLAGS)
> +wayland_scanner_LDADD = $(EXPAT_LIBS) $(LIBXML_LIBS) libwayland-util.la src/dtddata.o
>  pkgconfig_DATA += src/wayland-scanner.pc
>  
> +src/dtddata.o: protocol/wayland.dtd

Hi Peter,

I think the paths here may need some fixing. 'make distcheck' fails on

  CCLD     wayland-scanner
gcc: error: src/dtddata.o: No such file or directory
Makefile:1447: recipe for target 'wayland-scanner' failed

> +
> +%.o: %.S
> +	$(AM_V_GEN)$(CC) $(ASFLAGS) $(CPPFLAGS) $(TARGET_MACH) -c -o $@ $<
> +
>  if USE_HOST_SCANNER
>  wayland_scanner = wayland-scanner
>  else

> diff --git a/src/scanner.c b/src/scanner.c
> index f456aa5..45d60c1 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -25,6 +25,8 @@
>   * SOFTWARE.
>   */
>  
> +#include "config.h"
> +
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdarg.h>
> @@ -34,9 +36,18 @@
>  #include <expat.h>
>  #include <getopt.h>
>  #include <limits.h>
> +#include <unistd.h>
> +
> +#if HAVE_LIBXML
> +#include <libxml/parser.h>
> +#endif
>  
>  #include "wayland-util.h"
>  
> +/* Embedded wayland.dtd file, see dtddata.S */
> +extern char DTD_DATA_begin;
> +extern int DTD_DATA_len;
> +
>  enum side {
>  	CLIENT,
>  	SERVER,
> @@ -59,6 +70,56 @@ usage(int ret)
>  	exit(ret);
>  }
>  
> +static bool
> +is_dtd_valid(void)
> +{
> +	bool rc = true;
> +#if HAVE_LIBXML
> +	xmlParserCtxtPtr ctx = NULL;
> +	xmlDocPtr doc = NULL;
> +	xmlDtdPtr dtd = NULL;
> +	xmlValidCtxtPtr	dtdctx;
> +	xmlParserInputBufferPtr	buffer;
> +
> +	dtdctx = xmlNewValidCtxt();
> +	ctx = xmlNewParserCtxt();
> +	if (!ctx || !dtdctx)
> +		abort();
> +
> +	buffer = xmlParserInputBufferCreateMem(&DTD_DATA_begin,
> +					       DTD_DATA_len,
> +					       XML_CHAR_ENCODING_UTF8);
> +	if (!buffer) {
> +		fprintf(stderr, "Failed to init buffer for DTD.\n");
> +		abort();
> +	}
> +
> +	dtd = xmlIOParseDTD(NULL, buffer, XML_CHAR_ENCODING_UTF8);
> +	if (!dtd) {
> +		fprintf(stderr, "Failed to parse DTD.\n");
> +		abort();
> +	}
> +
> +	doc = xmlCtxtReadFd(ctx, STDIN_FILENO, "protocol", NULL, 0);

Using STDIN_FILENO misses an alternative use case of wayland-scanner,
where you pass both input and output file names as arguments.

See particularly the variable 'input' in main().

> +	if (!doc) {
> +		fprintf(stderr, "Failed to read XML\n");
> +		abort();
> +	}
> +
> +	rc = xmlValidateDtd(dtdctx, doc, dtd);
> +	xmlFreeDoc(doc);
> +	xmlFreeParserCtxt(ctx);
> +	xmlFreeValidCtxt(dtdctx);
> +	/* xmlIOParseDTD consumes buffer */
> +
> +	if (lseek(STDIN_FILENO, 0, SEEK_SET) != 0) {
> +		fprintf(stderr, "Failed to reset fd, output would be garbage.\n");
> +		abort();
> +	}
> +#endif
> +	return rc;
> +}

Otherwise the patch looks good to me.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151109/bd1aae61/attachment.sig>