Page MenuHomeFreeBSD

flua: Add libfetch functionality
Needs ReviewPublic

Authored by dch on May 25 2023, 8:20 PM.
Tags
None
Referenced Files
F175090628: D40274.id188941.diff
Thu, Oct 8, 4:57 AM
Unknown Object (File)
Wed, Oct 7, 4:33 PM
Unknown Object (File)
Wed, Oct 7, 12:02 PM
Unknown Object (File)
Wed, Oct 7, 11:50 AM
Unknown Object (File)
Wed, Oct 7, 11:49 AM
Unknown Object (File)
Wed, Oct 7, 11:49 AM
Unknown Object (File)
Wed, Oct 7, 11:48 AM
Unknown Object (File)
Wed, Oct 7, 11:47 AM

Details

Test Plan
lua

> f.parse_url("http:\\")
nil     Failed to parse URL

> r = f.parse_url("http://localhost/")
> for k,v in pairs(r) do; print('\t',k,v); end
                user
                scheme  http
                host    localhost
                doc     /
                password

> r = f.parse_url("http://user:pass@localhost:1999/rain?beret=raspberry")
> for k,v in pairs(r) do; print('\t',k,v); end
                user    user
                doc     /rain?beret=raspberry
                scheme  http
                host    localhost
                port    1999
                password        pass

> f.get_url("file:///var/run/motd", "/tmp/m")
true

> f.get_url("http://w3.org/", "/tmp/w")
true

> f.get_url("https://freebsd.org/", "/tmp/f")
true

> f.get_url("https://invalid.site/", "/tmp/i")
nil     Failed to read from URL: No error: 0

> f.get_url("https://wrong.host.badssl.com/", "/tmp/i")
SSL certificate subject doesn't match host wrong.host.badssl.com
nil     Failed to read from URL: Authentication error
>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 77798
Build 74681: arc lint + arc unit

Event Timeline

dch requested review of this revision.May 25 2023, 8:20 PM

i'd like to see some documentation for this.

This revision now requires changes to proceed.May 25 2023, 11:15 PM

baby can walk now

  • parse a url
  • fetch a file, fetch a url
libexec/flua/modules/lfetch.c
68 ↗(On Diff #122498)

My gut reaction to this is that we probably shouldn't expose the port at all if it's not explicitly set, just leave it nil in this table to allow for more idiomatic usage where you can simply test the port as truthy before using it.f

79 ↗(On Diff #122498)

Brace on its own line

83 ↗(On Diff #122498)

This should move up above the blank line

91 ↗(On Diff #122498)

IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of this function.

102 ↗(On Diff #122498)

Leaks file

105 ↗(On Diff #122498)

Ditto for these declarations; chunk_size I'd probably just make a #define since we're not going to be altering it

113 ↗(On Diff #122498)

Leaks fetch

clean up free, indentation.

Stop at fetchGet(), because fetchPutHTTP() is sadly not implemented

dch marked 7 inline comments as done.

clean up following review

f.parse_url("http:\\")

nil Failed to parse URL

r = f.parse_url("http://localhost/")
for k,v in pairs(r) do; print('\t',k,v); end

user
scheme  http
host    localhost
doc     /
password

r = f.parse_url("http://user:pass@localhost:1999/rain?beret=raspberry")
for k,v in pairs(r) do; print('\t',k,v); end

user    user
doc     /rain?beret=raspberry
scheme  http
host    localhost
port    1999
password        pass

f.get_url("file:///var/run/motd", "/tmp/m")

true

f.get_url("http://w3.org/", "/tmp/w")

true

f.get_url("https://freebsd.org/", "/tmp/f")

true

f.get_url("https://invalid/", "/tmp/i")

nil Failed to read from URL: No error: 0

f.get_url("https://womp.us/", "/tmp/i")

SSL certificate subject doesn't match host womp.us
nil Failed to read from URL: Authentication error

Clean up test formatting.

To Do:

add a manpage.

> f.parse_url("http:\\")
nil     Failed to parse URL
> r = f.parse_url("http://localhost/")
> for k,v in pairs(r) do; print('\t',k,v); end
                user
                scheme  http
                host    localhost
                doc     /
                password
> r = f.parse_url("http://user:pass@localhost:1999/rain?beret=raspberry")
> for k,v in pairs(r) do; print('\t',k,v); end
                user    user
                doc     /rain?beret=raspberry
                scheme  http
                host    localhost
                port    1999
                password        pass
> f.get_url("file:///var/run/motd", "/tmp/m")
true
> f.get_url("http://w3.org/", "/tmp/w")
true
> f.get_url("https://freebsd.org/", "/tmp/f")
true
> f.get_url("https://invalid.site/", "/tmp/i")
nil     Failed to read from URL: No error: 0
> f.get_url("https://wrong.host.badssl.com/", "/tmp/i")
SSL certificate subject doesn't match host wrong.host.badssl.com
nil     Failed to read from URL: Authentication error
>
dch edited the test plan for this revision. (Show Details)
dch marked an inline comment as not done.May 26 2023, 9:22 PM
dch added inline comments.
libexec/flua/modules/lfetch.c
113 ↗(On Diff #122498)

If I fclose(fetch); here, this segfaults:

Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC-Rio

f.get_url("https://invalid.site/", "/tmp/i")

fish: Job 1, '/usr/libexec/flua $argv' terminated by signal SIGSEGV (Address boundary error)

(lldb) thr backtrace all
* thread #1, name = 'flua', stop reason = signal SIGSEGV
  * frame #0: 0x00001252697ad0f9
    frame #1: 0xffadb0abacff9a89
libexec/flua/modules/lfetch.c
113 ↗(On Diff #122498)

The context for this one has changed, the comment was originally down in the error path in the loop. This branch looks fine

dch marked an inline comment as done.

add manpage, align with latest flua dir layout

bcr added a subscriber: bcr.

Man page looks good to me. The actual code has to be reviewed/accepted by someone else.

rebase & update for CURRENT

dch retitled this revision from flua: add libfetch functionality to flua: Add libfetch functionality.Wed, Oct 7, 6:54 AM
dch edited the summary of this revision. (Show Details)

libfetch has fetchLastErrString and fetchLastErrCode you can use to return more specific error information than errno provides.

Out of curiosity, what is the motivation for providing parse_url()?

For context/inspiration, I have a more complete libfetch module here: https://github.com/ryan-moeller/flualibs/tree/main/libfetch

Indeed, fetchPutHTTP() and fetchListHTTP() are not implemented, though fetchPut() or fetchList() do work for other protocols. I am also in the process of upstreaming some additions to libfetch adding an extended HTTP request API that supports such requests (I plan to move this to phab when I'm done getting my commit bit back): https://github.com/freebsd/freebsd-src/pull/1859

That is integrated into my flua module on a separate branch, including tests: https://github.com/ryan-moeller/flualibs/tree/fetchup/libfetch

Then it is possible to script typical REST API clients with flua. For example, this script creates an API client for a power distribution unit with authorization by a bearer token via a login endpoint: https://reviews.freebsd.org/P721

libexec/flua/libfetch/lfetch.c
79

No point closing NULL.

83

A bit inconsistent on the return style, please do a pass to add parens where missing.

86

Arguably you could just call fetchGetURL() with the url string and avoid needing to deal with the url struct here. fetchParseURL() should be setting error code/string for every parse failure reason (with url_seterr(URL_MALFORMED)), but some several errors fail silently today. Fixing that and using fetchLastErrCode/fetchLastErrString would get you adequate distinction between a malformed URL vs some other error.