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

jPath for self-closing XML tag includes slash #564

Closed
4 of 6 tasks
m-radzikowski opened this issue Apr 21, 2023 · 3 comments · Fixed by #595
Closed
4 of 6 tasks

jPath for self-closing XML tag includes slash #564

m-radzikowski opened this issue Apr 21, 2023 · 3 comments · Fixed by #595

Comments

@m-radzikowski
Copy link
Contributor

m-radzikowski commented Apr 21, 2023

  • Are you running the latest version? - yes, 4.2.2
  • Have you included sample input, output, error, and expected output?
  • Have you checked if you are using correct configuration?
  • Did you try online tool?

Description

jPath in updateTag() callback for self closing tag ends with /, which I doubt is intended.

Because of this, you need to check two different jPaths, with and without /, to accurately process tags that may be empty.

Input

<doc>
	<foo/>
	<foo>bar</foo>
	<lorem>ipsum</lorem>
</doc>

Code

const skip = ["doc.foo"];
const parser = new XMLParser({
	updateTag: (tagName, jPath) => {
		if (skip.includes(jPath)) {
			return false;
		}
		return tagName;
	},
});

Output

{
  "doc": {
    "foo": "",
    "lorem": "ipsum"
  }
}

expected data

{
  "doc": {
    "lorem": "ipsum"
  }
}

additional info

Code works if the skip array is changed to: ["doc.foo", "doc.foo/"] because for the self-closing tag jPath ends with /.
But I think that's easy to miss and not intended to be so? If you write code wanting to skip particular path and you don't test it with both self-closing and not-self closing tag you will be surprised.

Another workaround is to do:

	updateTag: (tagName, jPath) => {
		if (
			skip.includes(jPath) ||
			(jPath.endsWith("/") && skip.includes(jPath.slice(0, -1)))
		) {
			return false;
		}
		return tagName;
	},

Would you like to work on this issue?

  • Yes
  • No

Bookmark this repository for further updates. Visit SoloThought to know about recent features.

@github-actions
Copy link

We're glad you find this project helpful. We'll try to address this issue ASAP. You can vist https://solothought.com to know recent features. Don't forget to star this repo.

@amitguptagwl
Copy link
Member

I believe this problem exists with not only jPath but tagName as well. I believe it should be a small fix.

@m-radzikowski
Copy link
Contributor Author

No, actually, the tagName does not contain a trailing slash from what I've checked.

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

Successfully merging a pull request may close this issue.

2 participants