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

Set HTTP.NETRC if .netrc file found #1380

Closed
wants to merge 2 commits into from
Closed

Set HTTP.NETRC if .netrc file found #1380

wants to merge 2 commits into from

Conversation

magnusuMET
Copy link

Fixes #1299

@jswhit2
Copy link
Contributor

jswhit2 commented Oct 29, 2024 •
edited
Loading

this looks like a workaround for an issue in the netcdf-c library, correct? (or maybe a change in Ubuntu 22 to now have have CURLOPT_NETRC=CURL_NETRC_IGNORED by default)?

@magnusuMET
Copy link
Author

I think this is a new curl default as netcdf-c does not set any option related to the .netrc file. A better workaround might be to use .ncrc which can point to .netrc

@jswhit
Copy link
Collaborator

jswhit commented Oct 30, 2024

@DennisHeimbigner would appreciate your input on this

@DennisHeimbigner
Copy link
Collaborator

As noted, since using .netrc is off by default, it is necessary to have the HTTP.NETRC=<netrc file path>
set in .ncrc. Is this not working, or do you have a suggestion about changing this?

@magnusuMET
Copy link
Author

In versions prior to 1.6.1 .netrc was read by default and it broke on newer versions. I can't see any documentation for .netrc being read by default so this was an undocumented feature. I'd be fine with setting HTTP.NETRC in .ncrc

@jswhit
Copy link
Collaborator

jswhit commented Oct 31, 2024

@DennisHeimbigner maybe netcdf-c needs to set CURLOPT_NETRC to CURL_NETRC_OPTIONAL (looks like it is now set to CURL_NETRC_IGNORED by default on most systems)? Maybe if it is set to CURL_NETRC_IGNORED then setting HTTP.NETRC in .ncrc won't do anything.

@jswhit
Copy link
Collaborator

jswhit commented Oct 31, 2024

@magnusuMET can you provide a reproducible example of the problem that this PR fixes? (including server URL and auth info)

@DennisHeimbigner
Copy link
Collaborator

You are correct that the default is CURLOPT_NETRC_IGNORED.
If you specify a .netrc file using HTTP.NETRC, then CURLOPT_NETRC_OPTIONAL is used.
We could certainly change the default to be CURLOPT_NETRC_OPTIONAL, although
I am not sure what the consequences would be.

@magnusuMET
Copy link
Author

@jswhit
Server:

python3 -m http.server

Client code:

import netCDF4
url = "http://localhost:8000/"
netCDF4.Dataset(url)

Contents of .netrc:

machine localhost
    login foo
    password bar

Run the client code with envionment CURLOPT_VERBOSE=1. When .netrc is present it should have Authorization: Basic Zm9vOmJhcg== in the output. The current version python-netcdf does not contain cause this line to be emitted, and auth is not set. In any case the program will error since the server does not speak DAP

@jswhit2
Copy link
Contributor

jswhit2 commented Nov 1, 2024

I don't see that "Authorization" message, instead I get

* Host localhost:8000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:8000...
* Connected to localhost (::1) port 8000
* using HTTP/1.x
> GET /.dds HTTP/1.1
Host: localhost:8000
User-Agent: oc4.9.2
Accept: */*
Accept-Encoding: deflate, gzip, zstd

* Request completely sent off
* HTTP 1.0, assume close after body
< HTTP/1.0 404 File not found
< Server: SimpleHTTP/0.6 Python/3.12.6
< Date: Fri, 01 Nov 2024 22:07:36 GMT
< Connection: close
< Content-Type: text/html;charset=utf-8
< Content-Length: 335
<
* shutting down connection #0
syntax error, unexpected WORD_WORD, expecting SCAN_ATTR or SCAN_DATASET or SCAN_ERROR
context: <!DOCTYPE^ HTML><html lang="en"> <head> <meta charset="utf-8"> <title>Error response</title> </head> <body> <h1>Error response</h1> <p>Error code: 404</p> <p>Message: File not found.</p> <p>Error code explanation: 404 - Nothing matches the given URI.</p> </body></html>
Traceback (most recent call last):
  File "/Users/jwhitaker/test_netrc.py", line 3, in <module>
    netCDF4.Dataset(url)
  File "src/netCDF4/_netCDF4.pyx", line 2529, in netCDF4._netCDF4.Dataset.__init__
  File "src/netCDF4/_netCDF4.pyx", line 2166, in netCDF4._netCDF4._ensure_nc_success
OSError: [Errno -90] NetCDF: file not found: 'http://localhost:8000/

with or without the .netrc file present (using the mods in this PR).

@jswhit2
Copy link
Contributor

jswhit2 commented Nov 1, 2024

BTW, running _strencode on the Path object does not return bytes, I had to change that to from _strencode(path) to bytes(path)

@ocefpaf ocefpaf mentioned this pull request Nov 7, 2024
Open
1 task
@jswhit
Copy link
Collaborator

jswhit commented Nov 12, 2024 •
edited
Loading

I don't like the idea of including this since no one seems to understand if and why it's necessary. Users can always add this to their own code if they choose.

@magnusuMET
Copy link
Author

Since this was an undocumented feature of netcdf-c I agree the inclusion of this might be suprising for users. If this change is wanted I can instead open a PR against netcdf-c

@magnusuMET magnusuMET closed this Nov 13, 2024
@DennisHeimbigner
Copy link
Collaborator

It is documented in docs/auth.md

@magnusuMET
Copy link
Author

@DennisHeimbigner I see the following line:

If not specified, then libcurl will look first in the current directory, and then in the HOME directory.

I suppose this is an issue with netcdf-c and will raise an issue there

@DennisHeimbigner
Copy link
Collaborator

Sorry I have lost track. Can you re-state the Issue to be fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

netcdf > 1.6.1 does not read .netrc / .dodsrc
4 participants
@magnusuMET @jswhit2 @jswhit @DennisHeimbigner