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.

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

Suggestion: Allow http (and non :433) URLs for testing purposes #135

Closed
samlaycock opened this issue Aug 14, 2023 · 6 comments
Closed

Suggestion: Allow http (and non :433) URLs for testing purposes #135

samlaycock opened this issue Aug 14, 2023 · 6 comments

Comments

@samlaycock
Copy link

samlaycock commented Aug 14, 2023

Basically this:

import { connect } from '@planetscale/database'

const config = {
  url: 'http://user:password@localhost:3000'
}

const conn = connect(config)

Because....

We're using a home-grown Docker image to allow us to build with @planetscale/database while in offline situations (like the 14 hour flight I was just on)- it's a dumb express server that emulates, as far as we can tell, the current API this package uses. Obviously understanding the API isn't documented, is not really intended for this and is subject to change etc, it would still be good if this package didn't assume the connection URL is an https one when using the config.url value with a valid http/https string.

I've got a fork that implements it (and adds a test), but wanted to float the idea before submitting a PR.

@mattrobenolt
Copy link
Member

Tangentially related, I have a little hacked simulator that mirrors the API if you want to more reference this: https://github.com/mattrobenolt/ps-http-sim

I plan to produce something more official, but this works as a HTTP <> mysql proxy.

This sim would still have the exact same issue since it'd be listening over http as well.

I think we really want to prevent any potential insecure mistakes here, but maybe a config option such as allowInsecureConnection: true or something very explicit to allow an http url.

@iheanyi thoughts?

@iheanyi
Copy link
Member

iheanyi commented Aug 14, 2023

@mattrobenolt Let me think more on this, I think you're right though. The reason why we didn't have insecure connections is because the internal systems are always HTTPS and never over raw HTTP. But if there exists a way to use this driver over HTTP, then we should add a configuration setting for it.

@mattrobenolt
Copy link
Member

Yeah, the only situations are for something like local dev. It's never really been a priority since we don't provide a way to even run the driver against anything locally, but it seems folks are beating us to it. :)

The long term solution for a local simulator would very likely just listen on plaintext, so it'd likely make sense for us to provide some explicit option to do this.

I'd prefer we don't infer it based on the URL since that could be potentially a typo of http://.. so an explicit config option would be best imo.

@samlaycock
Copy link
Author

Appreciate this is an ongoing discussion.. this is what I have - #136

Feel free to disregard if you think there's a better way of handling it (it's a very naive approach)

@iheanyi
Copy link
Member

iheanyi commented Aug 21, 2023

This has been fixed as of version v1.11. You can use http in the url configuration setting and it will be respected :) Thanks for the report once again!

@iheanyi iheanyi closed this as completed Aug 21, 2023
@samlaycock
Copy link
Author

Awesome. Thanks for the quick work in getting this done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants