EventStore / EventStoreDB-Client-Rust

Rust EventStoreDB gRPC Client
MIT License
50 stars 13 forks source link

ClientSettings parse - allow underscore for hostname #164

Closed jsiefer closed 1 year ago

jsiefer commented 1 year ago

Can we allow underscore _ for host names when parsing settings string? Quite commonly used when using docker compose i think.

e.g. esdb://foo:bar@eventstore_db:2113

YoEight commented 1 year ago

Are you sure? I see everywhere that _ are prohibited

YoEight commented 1 year ago

url crate seems to accept _ indeed:

let is_invalid_host_char = |c| {
            matches!(
                c,
                '\0' | '\t'
                    | '\n'
                    | '\r'
                    | ' '
                    | '#'
                    | '/'
                    | ':'
                    | '<'
                    | '>'
                    | '?'
                    | '@'
                    | '['
                    | '\\'
                    | ']'
                    | '^'
                    | '|'
            )
        };
jsiefer commented 1 year ago

I think this part caused problems for me - inside the parsing function for ClientSettings

https://github.com/EventStore/EventStoreDB-Client-Rust/blob/15abe4006ee9f2e5ca4250756df5bc3daf34d53d/eventstore/src/grpc.rs#L361

YoEight commented 1 year ago

Yes but I used the url crate as a reference to know if underscores were actually supported because everywhere I see says otherwise

jsiefer commented 1 year ago

You are right. Looks like underscores are not allowed in domain names. I only stumbled on it when I was using docker and used an underscore in the service name.

Should (can) parsing fail then instead of falling back to default values?

YoEight commented 1 year ago

Hey @jsiefer,

PR #169 supports underscore in hostname and gossip seeds and also fixes the issue you mentioned.

jsiefer commented 1 year ago

thank you!