Page MenuHomeFreeBSD
Differential D40274 Diff 122501 libexec/flua/modules/lfetch.c

Changeset View

Standalone View

View Options

libexec/flua/modules/lfetch.c

Show All 33 Lines
#include <stdio.h> #include <stdio.h>
#include <string.h> #include <string.h>
#include <fetch.h> #include <fetch.h>
#include <lua.h> #include <lua.h>
#include "lauxlib.h" #include "lauxlib.h"
#include "lfetch.h" #include "lfetch.h"
#define CHUNK_SIZE 4096
/* /*
* Minimal implementation of libfetch * Minimal implementation of libfetch
*/ */
static int static int
lfetch_parse_url(lua_State *L) lfetch_parse_url(lua_State *L)
{ {
const char *url = luaL_checkstring(L, 1); const char *url = luaL_checkstring(L, 1);
struct url *u; struct url *u;
u = fetchParseURL(url); u = fetchParseURL(url);
if (u == NULL) { if (u == NULL) {
lua_pushnil(L); lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL"); lua_pushstring(L, "Failed to parse URL");
return (2); return (2);
} }
lua_newtable(L); lua_newtable(L);
lua_pushstring(L, u->scheme); lua_pushstring(L, u->scheme);
lua_setfield(L, -2, "scheme"); lua_setfield(L, -2, "scheme");
lua_pushstring(L, u->user); lua_pushstring(L, u->user);
lua_setfield(L, -2, "user"); lua_setfield(L, -2, "user");
lua_pushstring(L, u->pwd); lua_pushstring(L, u->pwd);
lua_setfield(L, -2, "password"); lua_setfield(L, -2, "password");
lua_pushstring(L, u->host); lua_pushstring(L, u->host);
kevansUnsubmitted
Done
Inline Actions

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

kevans: My gut reaction to this is that we probably shouldn't expose the port at all if it's not…
lua_setfield(L, -2, "host"); lua_setfield(L, -2, "host");
/* if port is not explicitly set, it defaults to 0, not scheme */ /* if port is not explicitly set, it defaults to 0, not scheme */
if (u->port != 0) {
lua_pushinteger(L, u->port); lua_pushinteger(L, u->port);
} else {
lua_pushnil(L);
}
lua_setfield(L, -2, "port"); lua_setfield(L, -2, "port");
/* doc is also known as the query string for http(s) */ /* for http(s), doc is the combined path & query string */
lua_pushstring(L, u->doc); lua_pushstring(L, u->doc);
lua_setfield(L, -2, "doc"); lua_setfield(L, -2, "doc");
kevansUnsubmitted
Done
Inline Actions

Brace on its own line

kevans: Brace on its own line
fetchFreeURL(u); fetchFreeURL(u);
return (1); return (1);
} }
kevansUnsubmitted
Done
Inline Actions

This should move up above the blank line

kevans: This should move up above the blank line
static int static int
lfetch_get_url(lua_State *L) { lfetch_get_url(lua_State *L)
{
const char *url = luaL_checkstring(L, 1); const char *url = luaL_checkstring(L, 1);
const char *out = luaL_checkstring(L, 2); const char *out = luaL_checkstring(L, 2);
struct url *u; struct url *u;
FILE *fetch;
FILE *file;
kevansUnsubmitted
Done
Inline Actions

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

kevans: IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of…
u = fetchParseURL(url); u = fetchParseURL(url);
if (u == NULL) { if (u == NULL) {
lua_pushnil(L); lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL"); lua_pushstring(L, "Failed to parse URL");
return (2); return (2);
} }
FILE *file = fopen(out, "w"); file = fopen(out, "w");
if (file == NULL) { if (file == NULL) {
fclose(file);
kevansUnsubmitted
Done
Inline Actions

Leaks file

kevans: Leaks `file`
fetchFreeURL(u); fetchFreeURL(u);
lua_pushnil(L); lua_pushnil(L);
lua_pushfstring(L, "Failed to open output file: %s", strerror(errno)); lua_pushfstring(L, "Failed to open output file: %s", strerror(errno));
kevansUnsubmitted
Done
Inline Actions

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

kevans: Ditto for these declarations; chunk_size I'd probably just make a `#define` since we're not…
return 2; return 2;
} }
FILE *fetch = fetchGet(u, ""); fetch = fetchGet(u, "");
if (fetch == NULL) { if (fetch == NULL) {
fclose(file); fclose(file);
fetchFreeURL(u); fetchFreeURL(u);
lua_pushnil(L); lua_pushnil(L);
kevansUnsubmitted
Done
Inline Actions

Leaks fetch

kevans: Leaks `fetch`
dchAuthorUnsubmitted
Done
Inline Actions

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
dch: If I `fclose(fetch);` here, this segfaults: Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC…
kevansUnsubmitted
Not Done
Inline Actions

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

kevans: The context for this one has changed, the comment was originally down in the error path in the…
lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno)); lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno));
return 2; return 2;
} }
int chunk_size = 4096; char buf[CHUNK_SIZE];
char buf[chunk_size];
size_t bytes; size_t bytes;
while ((bytes = fread(buf, 1, chunk_size, fetch)) > 0) { while ((bytes = fread(buf, 1, CHUNK_SIZE, fetch)) > 0) {
if (fwrite(buf, 1, bytes, file) != bytes) { if (fwrite(buf, 1, bytes, file) != bytes) {
fclose(fetch); fclose(fetch);
fclose(file); fclose(file);
fetchFreeURL(u); fetchFreeURL(u);
lua_pushnil(L); lua_pushnil(L);
lua_pushfstring(L, "Failed to write to file: %s", strerror(errno)); lua_pushfstring(L, "Failed to write to file: %s", strerror(errno));
return 2; return 2;
} }
Show All 23 Lines