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

Ads.txt file detection does not take mapped domains into account #48

Closed
helen opened this issue Nov 28, 2019 · 5 comments · Fixed by #131
Closed

Ads.txt file detection does not take mapped domains into account #48

helen opened this issue Nov 28, 2019 · 5 comments · Fixed by #131
Assignees
Milestone

Comments

@helen
Copy link
Contributor

helen commented Nov 28, 2019

Describe the bug
If you use a mapped domain (i.e. your wp-admin is on a different domain from your site, like on WordPress.com), the ads.txt file detection looks for the file on the wp-admin domain, which can return a false positive. See https://wordpress.org/support/topic/existing-ads-txt-file-found/

Steps to Reproduce
I haven't actually done this yet, but my guess is:

  1. Put an ads.txt in the root of a MS instance
  2. Set up domain mapping for a subsite
  3. Go to the ads.txt edit screen on the subsite
  4. See notice that an existing ads.txt file was found

Expected behavior
I would not expect to see the notice.

Screenshots
n/a

Environment information
This is as of Ads.txt 1.2.0, seen on WordPress.com with a business plan where you can install plugins and map domains.

Additional context
Reported here: https://wordpress.org/support/topic/existing-ads-txt-file-found/

@helen helen added this to the 1.2.1 milestone Nov 28, 2019
@helen
Copy link
Contributor Author

helen commented Nov 28, 2019

Looking at this a little bit, I think the technical approach might be something like:

  • Localize the admin.js script with homeURL as a var
  • Make adstxtUrl in admin.js use homeURL

I also wonder if it might be worth making that notice dismissible in case there are other false positive scenarios we haven't thought of yet, we'd still want to fix for those false positives but in the meantime at least people wouldn't keep getting this annoying and inaccurate message.

@soderlind
Copy link

@helen, agree. I use this method in my plugins to get the correct ajax url when running on a multisite and having a mapped domain:
https://github.com/soderlind/es6-wp-ajax-demo/blob/master/es6-wp-ajax-demo.php#L87-L96

Bet it work in this case also.

@peterwilsoncc
Copy link
Contributor

To reintroduce this filter and account for sites with the dashboard on a different domain to the website, the check will need to happen on the back end.

Testing client side causes CORS errors, preventing the JavaScript approach from determining the HTTP response code of /ads.txt across domains.

I think a server side check will be more robust as the plugin can rely on 301 redirects for sites using domain mapping without having to do function checks to determine if or which plugin is providing the feature.

@jeffpaul jeffpaul moved this from In Review to To Do in Open Source Practice Apr 19, 2022
@jeffpaul jeffpaul linked a pull request Mar 6, 2023 that will close this issue
4 tasks
@jeffpaul jeffpaul moved this from To Do to In Review in Open Source Practice Mar 6, 2023
@jeffpaul jeffpaul modified the milestones: Future Release, 1.5.0 Mar 6, 2023
@peterwilsoncc
Copy link
Contributor

I did some experiments with my earlier comment a while back but never committed anything as it was a little complicated.

For sites using a domain mapping plugin such as WordPress MU Domain Mapping and wasn't able to come up with a robust solution. Depending on the settings, the home_url() function can return different results if is_admin() === true.

As Multisite installs have supported domain mapping for some time, since WordPress 4.5, it's probably pretty safe to only support installs using the native features.

To avoid the CORS errors, we'd need to do something like:

add_action( 'wp_ajax_adstxts_check_for_existing_file', function() {
  $home_url_parsed = wp_parse_url( home_url() );
  if ( empty( $home_url_parsed['path'] ) ) {
     $check = wp_remote_request( home_url( '/ads.txt' ) )
     // Something to determine if it's a file, the WP managed file, etc
     // send result
     exit;
  }
  
  // Something to notify user that it can't be used on sites with the home in a sub_directory.
} );

@github-project-automation github-project-automation bot moved this from In Review to Merged in Open Source Practice Jun 5, 2023
@peterwilsoncc
Copy link
Contributor

I've merged in #131 to reintroduce a check for a real file for domain mapped sites.

  • It uses admin-ajax to request the ads.txt file from home_url( '/ads.txt' ) -- this avoids CORS errors
  • For sites using WordPress's built in domain mapping feature it just works(tm)
  • For sites using a domain mapping plugin, it relies on the plugin filtering home_url appropriately in the dashboard
  • A custom HTTP header is used to confirm the file is generated

@dkotter dkotter modified the milestones: 1.5.0, 1.4.3 Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
7 participants