Skip to content

Commit

Permalink
For 207 error handling, strclean the response and ensure it's a
Browse files Browse the repository at this point in the history
single-line. Only include path rather than full URIs.

* src/ne_207.c (end_element): ne_strclean the description cdata.
  (start_response): Only capture the path rather than the full URI.
  (handle_error): Build a single line comma-separated error string.
  (ne_simple_request): Use ne_xml_dispatch_request.

* test/twooh7.c, test/Makefile.in: New test case.
  • Loading branch information
notroj committed Dec 5, 2023
1 parent 9fabd80 commit 4998e4c
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 17 deletions.
33 changes: 17 additions & 16 deletions src/ne_207.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
WebDAV 207 multi-status response handling
Copyright (C) 1999-2021, Joe Orton <joe@manyfish.co.uk>
Copyright (C) 1999-2023, Joe Orton <joe@manyfish.co.uk>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
Expand Down Expand Up @@ -30,6 +30,7 @@
#include "ne_alloc.h"
#include "ne_utils.h"
#include "ne_xml.h"
#include "ne_xmlreq.h"
#include "ne_207.h"
#include "ne_uri.h"
#include "ne_basic.h"
Expand Down Expand Up @@ -171,7 +172,7 @@ end_element(void *userdata, int state, const char *nspace, const char *name)
case ELM_responsedescription:
if (HAVE_CDATA(p)) {
if (p->description) ne_free(p->description);
p->description = ne_strdup(cdata);
p->description = ne_strclean(ne_strdup(cdata));
}
break;
case ELM_href:
Expand Down Expand Up @@ -288,30 +289,33 @@ int ne_accept_207(void *userdata, ne_request *req, const ne_status *status)

/* This is passed as userdata to the 207 code. */
struct context {
char *href;
char *path;
ne_buffer *buf;
unsigned int is_error;
};

static void *start_response(void *userdata, const ne_uri *uri)
{
struct context *ctx = userdata;
if (ctx->href) ne_free(ctx->href);
ctx->href = ne_uri_unparse(uri);

/* Assume the only part of the URI that matters is the path, but
* could compare it to be sure. */
if (ctx->path) ne_free(ctx->path);
ctx->path = ne_strdup(uri->path);
return NULL;
}

static void handle_error(struct context *ctx, const ne_status *status,
const char *description)
{
if (status && status->klass != 2 && status->code != 424) {
if (ctx->is_error)
ne_buffer_czappend(ctx->buf, ", ");
ctx->is_error = 1;
ne_buffer_snprintf(ctx->buf, 512, "%s: %d %s\n",
ctx->href, status->code, status->reason_phrase);
if (description != NULL) {
/* TODO: these can be multi-line. Would be good to
* word-wrap this at col 80. */
ne_buffer_concat(ctx->buf, " -> ", description, "\n", NULL);
ne_buffer_snprintf(ctx->buf, 512, "%s: %d %s",
ctx->path, status->code, status->reason_phrase);
if (description) {
ne_buffer_concat(ctx->buf, " (", description, ")", NULL);
}
}

Expand All @@ -333,7 +337,6 @@ end_propstat(void *userdata, void *propstat,
}

/* Dispatch a DAV request and handle a 207 error response appropriately */
/* TODO: hook up Content-Type parsing; passing charset to XML parser */
int ne_simple_request(ne_session *sess, ne_request *req)
{
int ret;
Expand All @@ -355,10 +358,8 @@ int ne_simple_request(ne_session *sess, ne_request *req)

ne_207_set_response_handlers(p207, start_response, end_response);
ne_207_set_propstat_handlers(p207, NULL, end_propstat);

ne_add_response_body_reader(req, ne_accept_207, ne_xml_parse_v, p);

ret = ne_request_dispatch(req);
ret = ne_xml_dispatch_request(req, p);

if (ret == NE_OK) {
if (ne_get_status(req)->code == 207) {
Expand All @@ -380,7 +381,7 @@ int ne_simple_request(ne_session *sess, ne_request *req)
ne_207_destroy(p207);
ne_xml_destroy(p);
ne_buffer_destroy(ctx.buf);
if (ctx.href) ne_free(ctx.href);
if (ctx.path) ne_free(ctx.path);

ne_request_destroy(req);

Expand Down
1 change: 1 addition & 0 deletions test/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
/ssl
/xml
/stubs
/twooh7
/ca
/ca-stamp
/ssigned.pem
Expand Down
4 changes: 3 additions & 1 deletion test/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ BASIC_TESTS = uri-tests util-tests string-tests socket \
ZLIB_TESTS = compress
ZLIB_HELPERS = file1.gz file2.gz trailing.gz badcsum.gz truncated.gz \
corrupt1.gz corrupt2.gz empty.gz random.txt hello.txt hello.gz
DAV_TESTS = xml xmlreq oldacl acl3744 props lock
DAV_TESTS = xml xmlreq twooh7 oldacl acl3744 props lock
SSL_TESTS = socket-ssl ssl
SSL_HELPERS = ca-stamp
TESTS = @TESTS@
Expand Down Expand Up @@ -232,6 +232,8 @@ xml: xml.lo $(DEPS)
$(LINK) -o $@ xml.lo $(LDADD)
xmlreq: xmlreq.lo $(DEPS)
$(LINK) -o $@ xmlreq.lo $(LDADD)
twooh7: twooh7.lo $(DEPS)
$(LINK) -o $@ twooh7.lo $(LDADD)
lock: lock.lo $(DEPS)
$(LINK) -o $@ lock.lo $(LDADD)
largefile: largefile.lo $(DEPS)
Expand Down
126 changes: 126 additions & 0 deletions test/twooh7.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
Test cases for the ne_207.h interface.
Copyright (C) 2023, Joe Orton <joe@manyfish.co.uk>
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/

#include "config.h"

#include <sys/types.h>

#ifdef HAVE_STDLIB_H
#include <stdlib.h>
#endif
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif

#include "ne_207.h"

#include "tests.h"
#include "utils.h"

#define PFX "<?xml version='1.0' encoding='utf-8'?>\r\n"

#define RESP(status, rdesc) "<d:response>" \
"<d:href>http://localhost/container/resource3</d:href>" \
status rdesc \
"</d:response>"

#define MS_207_1(status, rdesc) PFX \
"<d:multistatus xmlns:d=\"DAV:\">" \
RESP(status, rdesc) \
"</d:multistatus>"

#define MS_207_2(s1, rd1, s2, rd2) PFX \
"<d:multistatus xmlns:d=\"DAV:\">" \
RESP(s1, rd1) \
RESP(s2, rd2) \
"</d:multistatus>"

static int simples(void)
{
static const struct {
int status;
const char *ctype;
const char *body;
const char *expect;
} ts[] = {
{ 207, "application/xml",
MS_207_1("<d:status>HTTP/1.1 423 Locked</d:status>", ""),
"423 Locked" },
{ 207, "application/xml",
MS_207_1("<d:status>HTTP/1.1 423 Locked</d:status>",
"<d:responsedescription>The thing was locked</d:responsedescription>"),
"The thing was" },
#if 0
{ 207, "application/xml",
MS_207_1("<d:status>HTTP/1.1 423 Locked</d:status>",
"<d:error><d:lock-token-submitted/></d:error>"),
"Resource locked" },
#endif
{ 207, "application/xml",
MS_207_2("<d:status>HTTP/1.1 423 Locked</d:status>",
"<d:responsedescription>The thing was locked</d:responsedescription>",
"<d:status>HTTP/1.1 404 Gone</d:status>",
"<d:responsedescription>No such thingy</d:responsedescription>"),
"such thingy" }
};
unsigned n;

for (n = 0; n < sizeof(ts)/sizeof(ts[0]); n++) {
char resp[1024];
ne_session *sess;
ne_request *req;
char *err;
int ret;

ne_snprintf(resp, sizeof resp,
"HTTP/1.1 %d OK\r\n"
"Content-Type: %s\r\n"
"Connection: close\r\n" "\r\n"
"%s", ts[n].status, ts[n].ctype, ts[n].body);

CALL(make_session(&sess, single_serve_string, resp));

req = ne_request_create(sess, "SIMPLE", "/");

ret = ne_simple_request(sess, req);
ONN("ne_simple_request didn't fail", ret == NE_OK);

err = ne_strclean(ne_strdup(ne_get_error(sess)));
ONV(strcmp(err, ne_get_error(sess)),
("error string wasn't cleaned: %s", ne_get_error(sess)));
NE_DEBUG(NE_DBG_HTTP, "test: got error string: %s\n", err);
ne_free(err);

ONV(strstr(ne_get_error(sess), ts[n].expect) == NULL,
("error string didn't match: '%s' - expected '%s'",
ne_get_error(sess), ts[n].expect));

ne_session_destroy(sess);
CALL(await_server());
}

return OK;
}

ne_test tests[] = {
T(simples),
T(NULL)
};

0 comments on commit 4998e4c

Please sign in to comment.