Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: Unexpected close tag #121

Closed
claviska opened this issue Dec 27, 2018 · 10 comments
Closed

Error: Unexpected close tag #121

claviska opened this issue Dec 27, 2018 · 10 comments

Comments

@claviska
Copy link
Contributor

claviska commented Dec 27, 2018

After upgrading to the latest 2.x version, I saw a strange error. After troubleshooting in the existing project, I decided to create a brand new project with only the following code and the same error persisted. (Note that the actual URL and credentials still work fine in 1.x).

const { createClient } = require('webdav');

const client = createClient(
  'http://example.com/webdav',
  {
    username: 'user',
    password: 'pass'
  }
);

// Get directory contents
client.getDirectoryContents('/').then(files => console.log(files));
(node:55105) UnhandledPromiseRejectionWarning: Error: Unexpected close tag
Line: 25
Column: 7
Char: >
    at error (/Users/claviska/Projects/webdav-test/node_modules/sax/lib/sax.js:651:10)
    at strictFail (/Users/claviska/Projects/webdav-test/node_modules/sax/lib/sax.js:677:7)
    at closeTag (/Users/claviska/Projects/webdav-test/node_modules/sax/lib/sax.js:871:9)
    at SAXParser.write (/Users/claviska/Projects/webdav-test/node_modules/sax/lib/sax.js:1436:13)
    at Parser.exports.Parser.Parser.parseString (/Users/claviska/Projects/webdav-test/node_modules/xml2js/lib/parser.js:322:31)
    at Parser.parseString (/Users/claviska/Projects/webdav-test/node_modules/xml2js/lib/parser.js:5:59)
    at /Users/claviska/Projects/webdav-test/node_modules/webdav/dist/interface/dav.js:30:16
    at new Promise (<anonymous>)
    at parseXML (/Users/claviska/Projects/webdav-test/node_modules/webdav/dist/interface/dav.js:29:12)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:55105) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:55105) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

It seems like an error parsing the WebDAV server's response, but given that 1.x still works fine I don't think it's anything to do with the server. I'm thinking it's probably related to the switch to Axios.

Thanks for your insight!

Node version: 10.2.1

@perry-mitchell
Copy link
Owner

perry-mitchell commented Dec 29, 2018

Hi @claviska - Sorry to hear!

This will most likely be in the update to the parsing of the XML response from your server. In v1 it was a bit more restrictive, which caused issues when attempting to parse responses from WebDAV servers like Seafile. It's possible there could be a problem with this mechanism.

I'd need to see the response from your server, however.. Would it be at all possible for you to share it? You could use node --inspect to possibly capture the network request.

EDIT: Sorry for the late response - Christmas holidays and such.

@claviska
Copy link
Contributor Author

claviska commented Dec 31, 2018

Thanks. I'm troubleshooting this more today. It looks all the other methods are working fine (e.g. getFileContents, moveFile, stat). The issue appears to be parsing the response in getDirectoryContents like you said.

Here's the axios request:

{ url: 'http://example-website.net/webdav',
  method: 'PROPFIND',
  headers:
   { Accept: 'text/plain',
     Depth: 1,
     Authorization: 'Basic ********' },
  responseType: 'text' }

And here's he axios response:

<!doctype html>
<html lang="en">
<head>
  <title>Untitled</title>
  <meta charset="utf-8">
  <meta name="description" content="Lorem ipsum dolor...">
</head>

<body>
  <h1>Untitled</h1>
  <p>Lorem ipsum dolor...</p>
</body>
</html>

That HTML is the source of the index.html that exists on the server. I'm not sure why the WebDAV server is returning the file contents instead of a proper XML response. If it matters, this is a test box on DreamHost with WebDAV enabled. (I wish I knew more about WebDAV servers, but I'm just implementing this as one of a number of protocols in my app.)

I'll keep digging and let you know what I find.

@claviska
Copy link
Contributor Author

I just learned that appending a trailing slash to the request returns the proper XML response:

http://example-website.net/webdav ==> HTML contents of file
http://example-website.net/webdav/ ==> WebDAV XML response

The trailing slash is stripped here, but it looks like url-join normalizes slashes on its own, so I'm not sure line 11 is necessary.

Do you foresee any issues removing that .replace()?

@perry-mitchell
Copy link
Owner

Do you foresee any issues removing that .replace()?

Hmm.. It's hard to say. It could potentially be a breaking change - I don't really see why the server would differentiate between the two - it's really the same path that a request is being made to. Normally when requesting URLs in the browser (say with a query string), no differentiation is actually made between say site.com/sub/?abc and site.com/sub?abc.. So why would WebDAV services return different results?

At least right now the current stable code in master works fine on a variety of services.. So I wonder if it's also something to do with specific config on your site. Are you running your own Apache/nginx config with any particular pathing rules?

@claviska
Copy link
Contributor Author

claviska commented Jan 2, 2019

I don't really see why the server would differentiate between the two - it's really the same path that a request is being made to.

I initially thought it's because the WebDAV-enabled directory is located in a subdirectory:

http://example-website.com/webdav/

I figured that when Apache receives a request without the trailing slash, it doesn't get passed to the WebDAV module and the web server would reply with the HTML as usual.

However, I just tried listing a subdirectory with and without the trailing slash and the same thing happens. 😕

So it seems with DreamHost's WebDAV, the only way you'll receive a WebDAV response for a directory listing is with a trailing slash.

So I wonder if it's also something to do with specific config on your site. Are you running your own Apache/nginx config with any particular pathing rules?

Again, this is a DreamHost test box that I've enabled WebDAV on. They have their own control panel, so the configs are all preset. I don't even have read access to view it, much less change it.

I'm going to open a support ticket with them. I'll let you know what I find.

@claviska
Copy link
Contributor Author

claviska commented Jan 2, 2019

My initial chat with DreamHost support resulted in them saying the trailing slash is required. I explained how it really shouldn't matter, and they've escalated the support request. 🤷‍♂️

@perry-mitchell
Copy link
Owner

@claviska After looking through the RFC (4918) for a while I found this:

There is a standing convention that when a collection is referred to
by its name without a trailing slash, the server MAY handle the
request as if the trailing slash were present. In this case, it
SHOULD return a Content-Location header in the response, pointing to
the URL ending with the "/". For example, if a client invokes a
method on http://example.com/blah (no trailing slash), the server may
respond as if the operation were invoked on http://example.com/blah/
(trailing slash), and should return a Content-Location header with
the value http://example.com/blah/. Wherever a server produces a URL
referring to a collection, the server SHOULD include the trailing
slash. In general, clients SHOULD use the trailing slash form of
collection names. If clients do not use the trailing slash form the
client needs to be prepared to see a redirect response.
Clients will find the DAV:resourcetype property more reliable than the URL to find
out if a resource is a collection.

Section 5.2 paragraph 8

I think that in this case it's appropriate that we force the ending slash for at least PROPFIND requests.

@claviska
Copy link
Contributor Author

claviska commented Jan 4, 2019

Great find...I didn't think to check the RFC.

Just to follow up, DreamHost seems to be set on it not being a config problem. They may be right, but there's a disconnect between support and engineering that makes it hard to tell.

I'm working on a PR and here are the only places PROPFIND appears to be used:

  • getDirectoryContents – easy fix, since this will always be a directory
  • quota – nothing to fix, since it uses the base URL + / already
  • stat – not so easy to fix, because we never know if we're working with a file or directory

I don't know what the best approach is for stat without sending another request. There are three possibilities I can think of:

  • List the parent directory first to determine if it's a file or directory (expensive)
  • Fetch using a trailing slash first, then fetch again on 404 (a bit dirty, but less expensive)
  • Don't append a slash and, for directories without it, hope the server response correctly with a 301 (DreamHost doesn't)

Any thoughts or alternatives?

@perry-mitchell
Copy link
Owner

I would create a helper to ensure a trailing slash - this could be used for getDirectoryContents and could replace the appended slash in quota.

As for stat, I would simply leave it as is. It's my opinion that it's up to the user to specify the correct path - but at the same time, the server really should understand that if a directory is called dir, both dir/ and dir should work for in a similar manner when fetching properties for that exact item.

Great summary @claviska - Looking forward to your PR! :)

@perry-mitchell
Copy link
Owner

Released in 2.2.1.

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

No branches or pull requests

2 participants