diesel icon indicating copy to clipboard operation
diesel copied to clipboard

DATABASE_URL parsing isn't very robust

Open Timmmm opened this issue 8 years ago • 5 comments

I haven't quite diagnosed this, but on Windows using PostgreSQL, this works:

diesel setup --database-url=postgres://postgres:myadminpass@localhost/my_db

But if I put DATABASE_URL=postgres://postgres:myadminpass@localhost/my_db in .env and run diesel setup it prints:

>diesel setup
Creating database: localhost:5432
could not translate host name "postgres" to address: Unknown host

That... doesn't seem right.

Timmmm avatar Jan 21 '17 16:01 Timmmm

Is this only the case with diesel_cli? Otherwise you should probably also open an issue at https://github.com/slapresta/rust-dotenv :)

Am 21.01.2017 um 17:44 schrieb Tim [email protected]:

I haven't quite diagnosed this, but on Windows using PostgreSQL, this works:

diesel setup --database-url=postgres://postgres:myadminpass@localhost/my_db

But if I put DATABASE_URL=postgres://postgres:myadminpass@localhost/my_db in .env and run diesel setup it prints:

diesel setup Creating database: localhost:5432 could not translate host name "postgres" to address: Unknown host

That... doesn't seem right.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

killercup avatar Jan 21 '17 17:01 killercup

Yeah I'm not sure. I'll build a little test app to try it.

Timmmm avatar Jan 21 '17 18:01 Timmmm

Actually scratch that, it does work. The problem was I also had an actual environment variable set and that apparently takes precedence over .env. It was set to postgresql://localhost:5432/. Seems like a bug in the URL parser.

In any case, this was a bit hard to diagnose. A simple way to make it a lot easier would be to print the URL that it is trying to connect, at least for diesel setup, and ideally fix the URL parsing.

Looks like this is the code.

Looks like it splits on / and gets the last component, which is why it failed. Why not do proper URL parsing? Here is some nice code I have lovingly crafted using Servo's url parsing crate.

/// Split a postgres URL into the address part and the database name, and appends "postgres" to the
/// end for some reason (TODO: seems weird; why?)
///
/// # Examples
///
/// ```
/// assert_eq!(split_pg_connection_string("postgres://user:pass@domain:port/database".to_string()),
///            Ok(("database".to_string(), "postgres://user:pass@domain:port/postgres".to_string()));
/// assert_eq!(split_pg_connection_string("postgres://user:pass@domain:port/database".to_string()),
///            Err(?));
/// ```
//#[cfg(feature = "postgres")]
fn split_pg_connection_string(database_url: &str) -> Result<(String, String), String> {

    let mut pg_url = Url::parse(database_url).map_err(|_| "URL parse error".to_string())?;
    
    if pg_url.scheme() != "postgres" && pg_url.scheme() != "postgresql" {
        return Err("Scheme must be postgres or postresql".to_string());
    }

    let database = {
        let path: Vec<_> = match pg_url.path_segments() {
            None => return Err("Scheme cannot have paths (this should never happen)".to_string()),
            Some(x) => x.collect(),
        };

        if path.len() != 1 || path[0].is_empty() {
            return Err("You must specify a single path element, e.g. postgres://.../database".to_string())
        }
        path[0].to_owned()
    };

    pg_url.set_path("postgres");

    Ok((database, pg_url.into_string()))
}

Also it would be nice if diesel setup reported the URL it tries to connect to.

Timmmm avatar Jan 21 '17 21:01 Timmmm

Just encountered this where diesel was choking on use of localhost in the DATABASE_URL without a good error message, only displaying "Can't connect to local MySQL server through socket '/tmp/mysql.sock' (2)". By randomly fiddling with things I noticed it started working when I changed this to 127.0.0.1 instead. I'm able to connect to the mysql server using another tool, Sqlectron using either 127.0.0.1 or localhost which is what I expected with diesel also.

Any particular reason why diesel wouldn't support localhost?

jcramb avatar May 05 '20 22:05 jcramb

@jcramb There is not much we can do here as we parse the connection string only in such a way that we extract hostname, username, password, database and port. After that everything is passed to libmysqlclient, where this error originates from. So that's not diesel does not support localhost but your version of libmysqlclient does not support it.

(Additionally: Your problem is not related to this old issue, as this one is about postgresql, not mysql.)

weiznich avatar May 06 '20 07:05 weiznich