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

XML and JSON file check broadness #275

Closed
pannal opened this issue Jul 8, 2024 · 10 comments
Closed

XML and JSON file check broadness #275

pannal opened this issue Jul 8, 2024 · 10 comments

Comments

@pannal
Copy link
Contributor

pannal commented Jul 8, 2024

Hey,

The current implementation of the XML and JSON validators look for filenames containing ".xml" or ".json", instead of checking whether the extension actually is the right one (endswith, or os.path.splitext()[1]).

This leads to issues with for example templating languages: I can't name my template "test.xml.tpl", or "test.xml.jinja2".

As those file would never even be "executed"/parsed as XML or JSON, I think the filename check is too broad.

@pannal pannal changed the title XML and JSON file check lenience XML and JSON file check broadness Jul 8, 2024
@razzeee
Copy link
Member

razzeee commented Jul 8, 2024

We still need a real xml file to do the schema validation https://github.com/xbmc/addon-check/tree/master/kodi_addon_checker/xml_schema

@pannal
Copy link
Contributor Author

pannal commented Jul 8, 2024

I don't understand. Why check a file for its XML schema, when the file extension isn't xml?

@razzeee
Copy link
Member

razzeee commented Jul 8, 2024

We obviously won't do that. But whatever get's commited to an addon repo, needs to be a xml - kodi or the repo servers doesn't understand jinja2 or other markups.

@pannal
Copy link
Contributor Author

pannal commented Jul 8, 2024

Well, in this specific case I'm using a jinja2-like templating language to dynamically build the XMLs I need, on plugin start. Because of the restrictions to the templating language in addons (no includes, no default variables) this is the only way to write reusable markup.

Are all files containing ".xml" in their filename validated, or just the ones explicitly in xml-containing folders, such as "resource/skins/"?

@pannal
Copy link
Contributor Author

pannal commented Jul 8, 2024

Nevertheless: I think the addon checker should check for the actual extension, not whether ".xml" or ".json" is inside the filename.

@razzeee
Copy link
Member

razzeee commented Jul 8, 2024

Are all files containing ".xml" in their filename validated, or just the ones explicitly in xml-containing folders, such as "resource/skins/"?

Only the addon.xml of your addon

addon_xml_path = os.path.join(addon_path, "addon.xml")

@razzeee
Copy link
Member

razzeee commented Jul 8, 2024

What you mean, is this check

if ".xml" in file["name"]:
and
if ".json" in file["name"]:
which are too broad

@pannal
Copy link
Contributor Author

pannal commented Jul 8, 2024

Yes. I mean those.

@pannal
Copy link
Contributor Author

pannal commented Jul 26, 2024

PR #276 should be correct for this.

@pannal
Copy link
Contributor Author

pannal commented Aug 16, 2024

Thanks!

@pannal pannal closed this as completed Aug 16, 2024
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

No branches or pull requests

2 participants