STORAGE_URL is not used globally: local.appspot.com is still hardcoded somewhere

[flomlo]

Hi,

the function locateService in line

rmfakecloud/internal/app/handlers.go

Line 522 in 3ff893c

host := config.DefaultHost

does not make use of the config parameter STORAGE_URL, but is instead hardcoded to local.appspot.com.

This means that every client of the rmfakecloud has to resolve the host local.appspot.com.
Whilst this poses no major problem for the remarkable device itself, it is a nuisance for 3rd-party cloud clients like rmcl, rmfuse, etc: The device running these clients must modify /etc/hosts and further accept the ca-certificate used to sign local.appspot.com.

thejonny has drafted a solution already, see #161.

It introduces STORAGE_HOST instead of STORAGE_URL (without a preceding https://) and allows us to run rmfakecloud without manually resolving local.appspot.com on the remarkable device (or the computer running rmfuse).

Whilst we are not sure of the precise use of STORAGE_URL in rmfakecloud it might be that is superseeded by STORAGE_HOST. But we are not sure yet.

[flomlo]

See rschroll/rmcl#9 for the changes to rmcl which enable it to properly use rmfakecloud.

[ddvk]

yep, I've only used rmapi which has the storage urls hardcoded, and only STORAGE_URL has to be correct

[flomlo]

I think deprecating STORAGE_URL in favour of STORAGE_HOST would be the cleanest (although API-changing). Or we calculate STORAGE_HOST from STORAGE_URL by removing preceding https:// or http://-strings (potentially ugly and error-prone?)

[nemunaire]

rmfakecloud could run either on HTTP and HTTPS, so deprecating STORAGE_URL in favor of a variable with less information is awkward.

Moreover, you can use the net/url go package to extract the scheme and host reliably.

For me STORAGE_HOST has to be determined from STORAGE_URL, and let to the user the choice to use http:// with software supporting it.

But 👍 for removing hardcoded local.appspot.com!

[flomlo]

That is a good objection, I didn't consider http://-only clients!

@ddvk: If I understand it correctly:
rmapi in the current version would break if this patch would be included and someone were to set STORAGE_URL other than local.appspot.com?
I guess that should be considered a bug and will be fixed at some point(?).
But a rmfakecloud where STORAGE_URL is not set and defaults to http[s]://local.appspot.com should continue to work with unpatched versions of rmapi.

Are you ok with including the net/url go package to reliably generate STORAGE_HOST from STORAGE_URL? Then we will modify the patch appropriately.

[TheJonny]

rmfakecloud could run either on HTTP and HTTPS, so deprecating STORAGE_URL in favor of a variable with less information is awkward.

Moreover, you can use the net/url go package to extract the scheme and host reliably.

For me STORAGE_HOST has to be determined from STORAGE_URL, and let to the user the choice to use http:// with software supporting it.

But +1 for removing hardcoded local.appspot.com!

Hi :)

can STORAGE_URL contain a subdirectory? or is the most general valid form $scheme://$host (i think $host could also contain a port)

does the remarkable support http or is it for use with the local forwarding proxy? there are queries that are answerd by the bare hostname, so i guess xochitl/sync adds "https://"

[TheJonny]

rmfakecloud could run either on HTTP and HTTPS, so deprecating STORAGE_URL in favor of a variable with less information is awkward.

you are right - that's why the PR is marked as draft :)

[nemunaire]

There is no more hardcoded local.appspot.com. I've just tested deleting the entry in /etc/hosts on my remarkable, and all the expected features continue to work well.