Arbitrary entities can break the settings form for XMLsitemap module

[jenlampton]

I know it seems unlikely, but I'm getting a multifield related db error on the xmlsitemap settings form:

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'multifield.id' in 'where clause': SELECT COUNT(*) AS expression FROM (SELECT 1 AS expression FROM {multifield} multifield WHERE (multifield.id > :db_condition_placeholder_0) AND (multifield.type = :db_condition_placeholder_1) ) subquery; Array ( [:db_condition_placeholder_0] => 0 [:db_condition_placeholder_1] => field_list_items ) in xmlsitemap_get_link_type_indexed_status() (line 962 of backdrop/modules/contrib/xmlsitemap/xmlsitemap.module).

However, disabling the multifield module brought the XMLsitemap admin form back to life.

See related: backdrop-contrib/multifield#12

[jenlampton]

I've noticed that there are a lot of entities appearing on the xmlsitemap settings form that do not have URLs, and should not have settings for xmlsitemap.

Is there a way to tell if an entity has a path, url, or uri, and exclude it if not? I've got both Flaggings and Multifields showing here now (though multifield is the only one causing this problem).

Some light reading:

• https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21Entity.php/function/Entity%3A%3Aurl/8.2.x
• https://api.drupal.org/api/drupal/includes%21common.inc/function/entity_uri/7.x

[alexfinnarn]

I'm not entirely sure I know where you are talking about, but if it is...

then https://github.com/backdrop-contrib/xmlsitemap/blob/1.x-1.x/xmlsitemap.module#L920 seems like you can alter away any entities you didn't want to have sitemap links for. If you look on line #901 they already explicitly disabled some entities, but I'd hope the alter hook can get you where you need to go.

As far as the URI callback, I don't see it mentioned in https://api.backdropcms.org/api/backdrop/core%21modules%21entity%21entity.api.php/function/hook_entity_info/1 as it is in https://api.drupal.org/api/drupal/modules%21system%21system.api.php/function/hook_entity_info/7.x. With that URI info, I would think you could ping something to determine if it was a public 200 response and exclude if not.

That's about all I can think of for now.

[jenlampton]

Okay, how about this: Instead of blindly adding XMLsitemap support for any arbitrary entity, we specifically include those we know are safe. Any contrib module that adds an entity can also add XMLsitemap support for it by implementing hook_xmlsitemap_link_info(). I think this is a much safer default than assuming all entities will have URIs, and will need XMLsitemap support.

I've filed a PR at #48 for review.

[alexfinnarn]

@jenlampton I took a look at this, and it makes sense and seems to function the same as the 1.x branch.

The only question I have is about hook_xmlsitemap_link_info(). In the API file, it lists a lot of keys in the returned array when the PR only has whatever entity_get_info() returns. That might be enough information, but I doubt it has the XMLSitemap specific parts...not sure.

It looks like xmlsitemap_get_link_info() is the only function that calls hook_xmlsitemap_link_info() and it adds defaults for the link types if they don't exist. Right?

I don't use this module currently, so I thought you might know more...thoughts?

[alexfinnarn]

I merged this in, but I will add a note on the next release. If someone included content of a module that didn't have hook_xmlsitemap_link_info() but had a URI, they would lose the inclusion once they updated the module.

That can be mitigated by adding a small custom module that implemented hook_xmlsitemap_link_info().