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.

Sign up for GitHub

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

Jump to bottom

d.o's GitLab instance 403s if User-Agent is absent or empty #1587

Merged
merged 1 commit into from Sep 1, 2023
Merged

d.o's GitLab instance 403s if User-Agent is absent or empty #1587

merged 1 commit into from Sep 1, 2023

Conversation

wimleers
Copy link
Member

@wimleers wimleers commented Sep 1, 2023 •
edited

Motivation
#1543 added acli app:new:from:drupal7. πŸ‘ Next steps in open sourcing: https://git.drupalcode.org/project/acquia_migrate/-/tree/recommendations is live.

So now let's get it all working together. Unfortunately, this made me hit an edge case in GitLab and/or PHP.

Ideally, we'd get the command to automatically download the latest, to allow you to omit --recommendations. But as an MVP, we can do:

./bin/acli app:new:from:drupal7 --directory=/tmp/foo --recommendations=https://git.drupalcode.org/project/acquia_migrate/-/raw/recommendations/recommendations.json --stored-analysis tests/fixtures/drupal7/training.acquia.com/extensions.json

Unfortunately, this is the output:

πŸ€– Scanning Drupal 7 site.
πŸ‘ Found Drupal 7 site (7.70 to be precise) at <location unknown>, with 96 modules enabled!
PHP Warning:  fopen(https://git.drupalcode.org/project/acquia_migrate/-/raw/recommendations/recommendations.json?ref_type=heads): Failed to open stream: HTTP request failed! HTTP/1.1 403 Forbidden.
 in /Users/wim.leers/cli/src/Command/App/NewFromDrupal7Command.php on line 151
…

Proposed changes

+    // PHP defaults to no user agent. (Drupal.org's) GitLab requires it.
+    // @see https://www.php.net/manual/en/filesystem.configuration.php#ini.user-agent
+    ini_set('user_agent', 'ACLI');
     $recommendations_resource = fopen($recommendations_location, 'r');

☝️ That's literally the whole PR! πŸ˜„

Alternatives considered
Impossible.

Testing steps

curl -A "" https://git.drupalcode.org/project/acquia_migrate/-/raw/recommendations/recommendations.json

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
 "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html>
  <head>
    <title>403 Forbidden.</title>
  </head>
  <body>
    <h1>Error 403 Forbidden.</h1>
    <p>Forbidden.</p>
    <h3>Error 54113</h3>
    <p>Details: cache-bru1480054-BRU 1693583048 3782263104</p>
    <hr>
    <p>Varnish cache server</p>
  </body>
</html>

vs

curl -A "ACLI" https://git.drupalcode.org/project/acquia_migrate/-/raw/recommendations/recommendations.json
{
    "data": [
        {
            "package": null,
……………………
}

@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (4affeb3) 91.76% compared to head (8e631e0) 91.76%.

Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #1587   +/-   ##
=========================================
  Coverage     91.76%   91.76%           
  Complexity     1809     1809           
=========================================
  Files           124      124           
  Lines          6470     6471    +1     
=========================================
+ Hits           5937     5938    +1     
  Misses          533      533
Files Changed Coverage Ξ”
src/Command/App/NewFromDrupal7Command.php 77.39% <100.00%> (+0.15%) ⬆️

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@wimleers wimleers self-assigned this Sep 1, 2023
@wimleers wimleers added the bug Something isn't working label Sep 1, 2023
@wimleers wimleers changed the title Set user agent prior to calling fopen(), to prevent a 403 response from d.o's GitLab instance d.o's GitLab instance 403s if User-Agent is absent or empty Sep 1, 2023
@wimleers wimleers mentioned this pull request Sep 1, 2023
Merged
@wimleers
Copy link
Member Author

wimleers commented Sep 1, 2023

Note: this is included in #1588, which literally is just one extra line change πŸ˜‡

@danepowell danepowell merged commit ad46983 into main Sep 1, 2023
23 of 24 checks passed
danepowell
danepowell reviewed Oct 17, 2023
View reviewed changes
src/Command/App/NewFromDrupal7Command.php
@@ -148,6 +148,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return Command::FAILURE;
}
}
// PHP defaults to no user agent. (Drupal.org's) GitLab requires it.
// @see https://www.php.net/manual/en/filesystem.configuration.php#ini.user-agent
ini_set('user_agent', 'ACLI');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guzzle HTTP client is the idiomatic way of making web requests in ACLI. For example:

cli/src/Command/Pull/PullCommandBase.php

Line 250 in 91306ec

$this->httpClient->request('GET', $downloadUrl, ['sink' => $localFilepath]);

I opened CLI-1160 and CLI-1161 to set the user agent for Guzzle requests and replace instances of fopen with Guzzle. Let me know if you anticipate any problems with that.

If there's some reason we can't use Guzzle, I'd prefer to set the user agent via a stream context for fopen rather than ini_set.

@danepowell danepowell deleted the gitlab-403 branch February 23, 2024 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danepowell danepowell danepowell left review comments

Assignees

@wimleers wimleers

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wimleers @danepowell