~martijnbraam/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 3

[PATCH readerview] gmni: fix building with bearssl-based gmni

Details
Message ID
<20210814230917.30357-1-zachdecook@librem.one>
DKIM signature
missing
Download raw message
Patch: +22 -15
Compare against https://git.sr.ht/~sircmpwn/gmni/commit/57064dd
---
 meson.build     |  3 ++-
 readerviewwin.c | 34 ++++++++++++++++++++--------------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/meson.build b/meson.build
index 30e1861..ac1cfdf 100644
--- a/meson.build
+++ b/meson.build
@@ -4,6 +4,7 @@ project('readerview', 'c',
gnome = import('gnome')
gtkdep = dependency('gtk4')
gmni = dependency('libgmni')
bearssl = meson.get_compiler('c').find_library('bearssl', required: true)

resources = gnome.compile_resources('readerview_resources', 'readerview.gresource.xml', source_dir: '.')

@@ -13,7 +14,7 @@ sources = [
  'readerviewwin.c',
]

executable('readerview', sources, resources, dependencies: [gtkdep, gmni], install: true)
executable('readerview', sources, resources, dependencies: [gtkdep, gmni, bearssl], install: true)

install_data(['org.postmarketos.readerview.desktop'],
	     install_dir: get_option('datadir') / 'applications')
diff --git a/readerviewwin.c b/readerviewwin.c
index 9fbbe90..8d8b5d4 100644
--- a/readerviewwin.c
+++ b/readerviewwin.c
@@ -283,9 +283,6 @@ readerview_app_window_class_init (ReaderviewAppWindowClass *class)
ReaderviewAppWindow *
readerview_app_window_new (ReaderviewApp *app)
{
	SSL_load_error_strings();
	ERR_load_crypto_strings();

	return g_object_new (READERVIEW_APP_WINDOW_TYPE, "application", app, NULL);
}

@@ -431,6 +428,17 @@ readerview_app_window_update_tofu_icon()
	gtk_entry_set_icon_from_paintable(GTK_ENTRY(appwin->url), GTK_ENTRY_ICON_PRIMARY, GDK_PAINTABLE(tofu_icon));
}

static int
resp_read(void *state, void *buf, size_t nbyte)
{
	struct gemini_response *resp = state;
	if (resp->sc) {
		return br_sslio_read(&resp->body, buf, nbyte);
	} else {
		return read(resp->fd, buf, nbyte);
	}
}

void
readerview_app_window_display_response_text_gemini(struct gemini_response *resp, GtkBox *target)
{
@@ -440,7 +448,7 @@ readerview_app_window_display_response_text_gemini(struct gemini_response *resp,
	struct gemini_parser p;
	struct gemini_token tok;
	enum gemini_tok last_tok;
	gemini_parser_init(&p, resp->bio);
	gemini_parser_init(&p, &resp_read, resp);
	temp_url = curl_url();


@@ -566,11 +574,11 @@ readerview_app_window_display_response_text_plain(struct gemini_response *resp,
	int contents_length = 0;

	for (int n = 1; n > 0;) {
		n = BIO_read(resp->bio, &contents[contents_length], contents_size - contents_length - 1);
		if (n == -1) {
		n = br_sslio_read(&resp->body, &contents[contents_length], contents_size - contents_length - 1);
		/*if (n == -1) {
			fprintf(stderr, "Error: read");
			return;
		}
		}*/
		contents_length += n;
		if(contents_length > contents_size - 2) {
			contents_size *= 2;
@@ -607,11 +615,11 @@ readerview_app_window_display_response_image(struct gemini_response *resp, GtkBo
	int contents_length = 0;

	for (int n = 1; n > 0;) {
		n = BIO_read(resp->bio, &contents[contents_length], contents_size - contents_length - 1);
		if (n == -1) {
		n = br_sslio_read(&resp->body, &contents[contents_length], contents_size - contents_length - 1);
		/*if (n == -1) {
			fprintf(stderr, "Error: read");
			return;
		}
		}*/
		contents_length += n;
		if(contents_length > contents_size - 2) {
			contents_size *= 2;
@@ -681,11 +689,10 @@ on_redirect:
	GtkEntryBuffer *textbuf = gtk_entry_get_buffer(urlbar);
	gtk_entry_buffer_set_text(textbuf, url, strlen(url));

	opts.ssl_ctx = SSL_CTX_new(TLS_method());
	gemini_tofu_init(&cfg.tofu, opts.ssl_ctx, &tofu_callback, &cfg);
	gemini_tofu_init(&cfg.tofu, &tofu_callback, &cfg);

	struct gemini_response resp;
	enum gemini_result r = gemini_request(url, &opts, &resp);
	enum gemini_result r = gemini_request(url, &opts, &cfg.tofu, &resp);
	if (r != GEMINI_OK) {
		fprintf(stderr, "Error: %s\n", gemini_strerr(r, &resp));
		readerview_app_window_display_error((char *)gemini_strerr(r, &resp), target);
@@ -728,7 +735,6 @@ on_redirect:
		break;
	}
	gemini_response_finish(&resp);
	SSL_CTX_free(opts.ssl_ctx);
	gemini_tofu_finish(&cfg.tofu);
}

-- 
2.32.0
Details
Message ID
<20210815135447.GB6626@gmail.com>
In-Reply-To
<20210814230917.30357-1-zachdecook@librem.one> (view parent)
DKIM signature
pass
Download raw message
> @@ -566,11 +574,11 @@ readerview_app_window_display_response_text_plain(struct gemini_response *resp,
>  	int contents_length = 0;
>  
>  	for (int n = 1; n > 0;) {
> -		n = BIO_read(resp->bio, &contents[contents_length], contents_size - contents_length - 1);
> -		if (n == -1) {
> +		n = br_sslio_read(&resp->body, &contents[contents_length], contents_size - contents_length - 1);
> +		/*if (n == -1) {
>  			fprintf(stderr, "Error: read");
>  			return;
> -		}
> +		}*/

This is maybe silly question, but why to commit comented code?

>  		contents_length += n;
>  		if(contents_length > contents_size - 2) {
>  			contents_size *= 2;
Details
Message ID
<CDPAOPWKT8AE.37MQ80YXQFUN9@pine64-pinephone>
In-Reply-To
<20210815135447.GB6626@gmail.com> (view parent)
DKIM signature
missing
Download raw message
On Sun Aug 15, 2021 at 9:54 AM EDT, Jiri Vlasak wrote:
> > +		n = br_sslio_read(&resp->body, &contents[contents_length], contents_size - contents_length - 1);
> > +		/*if (n == -1) {
>
> This is maybe silly question, but why to commit comented code?
>

Definitely not a silly question, thanks for asking.

Umm, soo the way I developed this patch was in two steps:

1. Commenting out code until it compiled
2. Modifying commented until it worked

I didn't read the documentation for br_sslio_read. If it returns -1 on failure, then this should be uncommented.

So this patch isn't really production-worthy,
but there's also another reason why it isn't:

According to (https://lists.sr.ht/~sircmpwn/gmni-devel/%3CC9OP3QNDE38Y.2X56ETVQZRR7L%40taiga%3E),
clients using libgmni should have no need to interact with the ssl implementation directly.
This is not done yet.

We need to 

1. Expose relevant functions in libgmni
2. refactor gmni and gmnlm to use those functions
3. refactor readerview to use those functions instead of openssl/bearssl ones.
Details
Message ID
<CDPW9IOU09CI.BM1FYS0ZSUAS@taiga>
In-Reply-To
<CDPAOPWKT8AE.37MQ80YXQFUN9@pine64-pinephone> (view parent)
DKIM signature
pass
Download raw message
On Sat Aug 21, 2021 at 5:27 PM CEST, Zach DeCook wrote:
> On Sun Aug 15, 2021 at 9:54 AM EDT, Jiri Vlasak wrote:
> > > +		n = br_sslio_read(&resp->body, &contents[contents_length], contents_size - contents_length - 1);
> > > +		/*if (n == -1) {
> >
> > This is maybe silly question, but why to commit comented code?
> >
>
> Definitely not a silly question, thanks for asking.
>
> Umm, soo the way I developed this patch was in two steps:
>
> 1. Commenting out code until it compiled
> 2. Modifying commented until it worked

This is not how you write robust code.

> I didn't read the documentation for br_sslio_read.

This is also not how you write robust code.
Reply to thread Export thread (mbox)