Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes xml strings that aren't closed #113

Closed
wants to merge 1 commit into from

3 participants

@tsgautier

The parser never returns if given strings like "" when calling parseString() because the sax close() method was never called.

This fixes that by calling close on the sax write stream.

Note that the unit test that covers throwing an error synchronously was in my opinion actually wrong, since an error generated synchronously does and should bubble up immediately - so I flipped the logic of the unit test and added an appropriate comment.

@drzax

Is this likely to be merged soon? It fixes #51 which has been hanging around for a while and passes the unit tests.

@Leonidas-from-XIV

This PR somehow slipped my attention. Will check it out.

@actionshrimp actionshrimp referenced this pull request in raoulmillais/node-7digital-api
Closed

Hangs if incomplete XML returned by API #21

@Leonidas-from-XIV Leonidas-from-XIV closed this pull request from a commit
@Leonidas-from-XIV Merge branch 'tsgautier-master'
Also added tsgautier to contributors.

Closes #113
Closes #51
ad05a02
@Leonidas-from-XIV

Thanks for the pull request, just merged it and it is in the current maintenance release, 0.4.1. Happy new year y'all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 6, 2013
  1. @tsgautier

    ensure that parser is closed when parsing a string, to guarantee well…

    tsgautier authored
    … formed but not closed XML generates an error
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 4 deletions.
  1. +1 −1  lib/xml2js.js
  2. +1 −1  src/xml2js.coffee
  3. +10 −2 test/parser.test.coffee
View
2  lib/xml2js.js
@@ -359,7 +359,7 @@
this.emit("end", null);
return true;
}
- return this.saxParser.write(bom.stripBOM(str.toString()));
+ return this.saxParser.write(bom.stripBOM(str.toString())).close();
};
return Parser;
View
2  src/xml2js.coffee
@@ -299,7 +299,7 @@ class exports.Parser extends events.EventEmitter
@emit "end", null
return true
- @saxParser.write bom.stripBOM str.toString()
+ @saxParser.write(bom.stripBOM str.toString()).close()
exports.parseString = (str, a, b) ->
# let's determine what we got as arguments
View
12 test/parser.test.coffee
@@ -285,8 +285,10 @@ module.exports =
throw new Error 'error throwing in callback'
throw new Error 'error throwing outside'
catch e
- # don't catch the exception that was thrown by callback
- equ e.message, 'error throwing outside'
+ # the stream is finished by the time the parseString method is called
+ # so the callback, which is synchronous, will bubble the inner error
+ # out to here, make sure that happens
+ equ e.message, 'error throwing in callback'
test.finish()
'test xmlns': skeleton(xmlns: true, (r) ->
@@ -340,3 +342,9 @@ module.exports =
equ err, null
assert.deepEqual resWithNew, resWithoutNew
test.done()
+
+ 'test not closed but well formed xml': (test) ->
+ xml = "<test>"
+ xml2js.parseString xml, (err, parsed) ->
+ assert.equal err.message, 'Unclosed root tag\nLine: 0\nColumn: 6\nChar: '
+ test.finish()
Something went wrong with that request. Please try again.