Repository: sfackler/rust-postgres (language: rust)

Issue:
windows connection error UTF-8
I setup a postgresql server v13 on my window (mostly to see if it was possible) all default expect for ssl configuration (that work finally) but I end up with a hard error:

```none
Jul 11 01:49:50.720 TRACE mio::poll: registering event source with poller: token=Token(2), interests=READABLE | WRITABLE
Jul 11 01:49:50.780 TRACE tokio_util::codec::framed_impl: flushing framed transport
Jul 11 01:49:50.780 TRACE tokio_util::codec::framed_impl: writing; remaining=44
Jul 11 01:49:50.781 TRACE tokio_util::codec::framed_impl: framed transport flushed
Jul 11 01:49:50.782 TRACE tokio_util::codec::framed_impl: attempting to decode a frame
Jul 11 01:49:50.782 TRACE tokio_util::codec::framed_impl: frame decoded from buffer
Jul 11 01:49:50.783 TRACE tokio_util::codec::framed_impl: flushing framed transport
Jul 11 01:49:50.784 TRACE tokio_util::codec::framed_impl: writing; remaining=81
Jul 11 01:49:50.784 TRACE tokio_util::codec::framed_impl: framed transport flushed
Jul 11 01:49:50.784 TRACE tokio_util::codec::framed_impl: attempting to decode a frame
Jul 11 01:49:50.784 TRACE tokio_util::codec::framed_impl: attempting to decode a frame
Jul 11 01:49:50.785 TRACE tokio_util::codec::framed_impl: frame decoded from buffer
Jul 11 01:49:51.027 TRACE tokio_util::codec::framed_impl: flushing framed transport
Jul 11 01:49:51.027 TRACE tokio_util::codec::framed_impl: writing; remaining=181
Jul 11 01:49:51.027 TRACE tokio_util::codec::framed_impl: framed transport flushed
Jul 11 01:49:51.028 TRACE tokio_util::codec::framed_impl: attempting to decode a frame
Jul 11 01:49:51.038 TRACE tokio_util::codec::framed_impl: attempting to decode a frame
Jul 11 01:49:51.039 TRACE tokio_util::codec::framed_impl: frame decoded from buffer
Jul 11 01:49:51.039 TRACE mio::poll: deregistering event source from poller
Jul 11 01:49:51.039 DEBUG optomata::internal_server_error: Pool { source: Backend(Error { kind: Parse, cause: Some(Custom { kind: InvalidInput, error: Utf8Error { valid_up_to: 34, error_len: Some(1) } }) }) }
Jul 11 01:49:51.040 ERROR optomata::internal_server_error: Pool: An error occured while creating a new object: error parsing response from server: invalid utf-8 sequence of 1 bytes from index 34
```

I use deadpool but that unrelated, I'm unaware of a PostgreSQL option for internal protocol encoding. The same code work for a linux server.

HS: don't do like me it's a nightmare

I did run the test of tokio-postgresql:

```none
---- custom_array stdout ----
thread 'custom_array' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Db, cause: Some(DbError { severity: "ERREUR", parsed_severity: Some(Error), code: SqlState(E42704), message: "le type « hstore[] » n'existe pas", detail: None, hint: None, position: Some(Original(12)), where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("d:\\pginstaller_13.auto\\postgres.windows-x64\\src\\backend\\parser\\parse_type.c"), line: Some(274), routine: Some("typenameType") }) }', tokio-postgres\tests\test\main.rs:193:62
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- md5_password_ok stdout ----
thread 'md5_password_ok' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Parse, cause: Some(Custom { kind: InvalidInput, error: Utf8Error { valid_up_to: 4, error_len: Some(1) } }) }', tokio-postgres\tests\test\main.rs:32:53

---- notices stdout ----
thread 'notices' panicked at 'assertion failed: `(left == right)`
  left: `"l'identifiant « xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx » sera tronqué en « xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx »"`,
 right: `"identifier \"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\" will be truncated to \"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\""`', tokio-postgres\tests\test\main.rs:604:5

---- md5_password_wrong stdout ----
thread 'md5_password_wrong' panicked at 'error parsing response from server: invalid utf-8 sequence of 1 bytes from index 4', tokio-postgres\tests\test\main.rs:73:19

---- pipelined_prepare stdout ----
thread 'pipelined_prepare' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Db, cause: Some(DbError { severity: "ERREUR", parsed_severity: Some(Error), code: SqlState(E42704), message: "le type « hstore[] » n'existe pas", detail: None, hint: None, position: Some(Original(12)), where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("d:\\pginstaller_13.auto\\postgres.windows-x64\\src\\backend\\parser\\parse_type.c"), line: Some(274), routine: Some("typenameType") }) }', tokio-postgres\tests\test\main.rs:111:66

---- plain_password_ok stdout ----
thread 'plain_password_ok' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Parse, cause: Some(Custom { kind: InvalidInput, error: Utf8Error { valid_up_to: 4, error_len: Some(1) } }) }', tokio-postgres\tests\test\main.rs:32:53

---- plain_password_wrong stdout ----
thread 'plain_password_wrong' panicked at 'error parsing response from server: invalid utf-8 sequence of 1 bytes from index 4', tokio-postgres\tests\test\main.rs:51:19

---- scram_password_wrong stdout ----
thread 'scram_password_wrong' panicked at 'error parsing response from server: invalid utf-8 sequence of 1 bytes from index 4', tokio-postgres\tests\test\main.rs:95:19

---- scram_password_ok stdout ----
thread 'scram_password_ok' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Parse, cause: Some(Custom { kind: InvalidInput, error: Utf8Error { valid_up_to: 4, error_len: Some(1) } }) }', tokio-postgres\tests\test\main.rs:32:53

---- types::test_hstore_params stdout ----
thread 'types::test_hstore_params' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Db, cause: Some(DbError { severity: "ERREUR", parsed_severity: Some(Error), code: SqlState(E42704), message: "le type « hstore » n'existe pas", detail: None, hint: None, position: Some(Original(16)), where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("d:\\pginstaller_13.auto\\postgres.windows-x64\\src\\backend\\parser\\parse_type.c"), line: Some(274), routine: Some("typenameType") }) }', tokio-postgres\tests\test\types\mod.rs:46:14

---- types::test_citext_params stdout ----
thread 'types::test_citext_params' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Db, cause: Some(DbError { severity: "ERREUR", parsed_severity: Some(Error), code: SqlState(E42704), message: "le type « citext » n'existe pas", detail: None, hint: None, position: Some(Original(87)), where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("d:\\pginstaller_13.auto\\postgres.windows-x64\\src\\backend\\parser\\parse_type.c"), line: Some(274), routine: Some("typenameType") }) }', tokio-postgres\tests\test\types\mod.rs:276:10
```

```sql
SHOW SERVER_ENCODING;
```

```none
result to:
 server_encoding
-----------------
 UTF8
(1 ligne)
```

I have no idea if support windows is a goal or not.


Discussion / hints:
Can you use something like Wireshark to capture the communications with the postgres server?
[output.pcapng.gz](https://github.com/sfackler/rust-postgres/files/6796159/output.pcapng.gz)

I tried notice I don't know much about how to use wireshark, I filter based on localloop and port 5432, there is no sensitive information, well in fact there was, I forget to update the password, the old one was a generate password from a old server deleted... and so using the good password there is not anymore the error. So I guess the bug is somewhere for wrong password code.

wrong password was = "Mo&NTSgsOR;0[2$,lKXH" (not used anymore so safe)
It looks like the connection was made with TLS, so I unfortunately can't see any of the interesting parts. Could you capture the same thing but with TLS disabled?
Oh, yes, of course, so finally my password wasn't compromise.

here, the real password is.... "password", and I put a wrong password "passwor" same error
[output.pcapng.gz](https://github.com/sfackler/rust-postgres/files/6797262/output.pcapng.gz)

It looks like the issue is with Postgres's French locale. The error message is not valid UTF-8:

Raw escaped bytes:
```
\x61\x75\x74\x68\x65\x6e\x74\x69\x66\x69\x63\x61\x74\x69\x6f
\x6e\x20\x70\x61\x72\x20\x6d\x6f\x74\x20\x64\x65\x20\x70\x61\x73
\x73\x65\x20\xe9\x63\x68\x6f\x75\xe9\x65\x20\x70\x6f\x75\x72\x20
\x6c\x27\x75\x74\x69\x6c\x69\x73\x61\x74\x65\x75\x72\x20\x20\xab
\x20\x6f\x70\x74\x6f\x6d\x61\x74\x61\x20\xbb\x00
```

`String::from_utf8_lossy` decode:

```
authentification par mot de passe �chou�e pour l'utilisateur  � optomata �
```

Note the `�`s, indicating an invalid UTF-8 byte sequence.
probably WTF-16 of windows, should this be considered a bug to report https://www.postgresql.org/docs/13/bug-reporting.html#id-1.3.8.7 ? In my option yes, postgresql should return a utf8 string in its internal protocol even if windows use WTF-16.
Yeah, this definitely seems like a bug in Postgres. Error messages should respect client_encoding, or if not their encoding should be specified in the [documentation](https://www.postgresql.org/docs/13/protocol-error-fields.html).
Hi,

As I just answered on the postgres mailing lists (https://www.postgresql.org/message-id/flat/CAO16vDNM5_eUZG3unbumx2AJt11VZxuQxn9cSZ9VhSCpD1TMgw%40mail.gmail.com) , this is in my understanding sadly working as designed.  The client encoding can't be set during startup (and authentication is part of it), see https://github.com/postgres/postgres/blob/master/src/backend/utils/mb/mbutils.c#L85-L88 for more details about it:

> /*
>  * During backend startup we can't set client encoding because we (a)
>  * can't look up the conversion functions, and (b) may not know the database
>  * encoding yet either.  So SetClientEncoding() just accepts anything and
>  * remembers it for InitializeClientEncoding() to apply later.
>  */

So rust-postgres should be prepared to receive non UTF-8 messages until authentication succeeded.
Hmm, that's unfortunate. I guess we'll have to add some logic to our handling of ErrorResponses to do a lossy UTF-8 decode as necessary.
Indeed.  I'm also wondering how much relying on UTF-8 only encoding can be problematic in some parts of the world.  I think that at least Japan is still using a lot of those encodings, and probably Taiwan with big5 encoding, and I'm not sure that all conversion are supported.
> such messages are currently sent using the language and encoding derived from the postmaster's environment. In simple cases that'll be the same as the client's environment and everything works.

I have strong opinion about all should default to UTF-8 in 2021, the specification of the protocol should specified a static encoding at least until the client can choice one. The current behavior to "use what OS use" that impact the protocol is not ideal cause the client need to handle every possible case like handle big5 encoding (never hear of it before but anyway). Asking a client to handle every text encoding is annoying.

But I see no reason to not have a flexible call to `from_utf8_lossy` in the error handling in Rust at least for now even if this allocate a string I don't see this as a big problem for an error return.
> I  have strong opinion about all should default to UTF-8 in 2021

In an ideal world I would agree, but I think that it would break on a lot of environments as there are a lot of UTF-8 characters that are not supported in other encodings.  And I saw how creative some people are naming roles, databases and other objects :(

> But I see no reason to not have a flexible call to from_utf8_lossy in the error handling in Rust at least for now even if this allocate a string I don't see this as a big problem for an error return.

+1
Hi there,

@sfackler: Looks like I have a fix. 

I'm on a Windows machine (german locale), with a local PostgreSQL database and a rust app I'm currently developing, using sqlx. Upon connection, I'm getting pretty much the same error message as above. My hunch is that everyone with connection issues in a non-us locale having 'funky' characters in their error message will get this issue. For me, it couldn't find the database and used non-ascii quote chars to enclose the database name - but this could happen in any error message, frankly.

I debugged this a little into the postgres-protocol crate (my toko-postgres is using the newest version 0.6.6). 

@sfackler you were talking about upgrading the error handling to be able to handle this better, and this is what I've done. So when parsing an `ErrorField` I now use `String::from_utf8_lossy()` (which returns `Cow<'a,str>`) instead of `get_str()`. Also, `ErrorField`'s `value` field is now `Cow<'a,str>` instead of `&str`. With this in place, I see proper error messages now (and I could finally find out what the connection problem in my project was :)

Please let me know if the fix is acceptable, or if I should change anything. Currently the fix sits in my fork here:
https://github.com/upachler/rust-postgres/tree/bugfix/803-windows-connection-error-utf-8 

Thanks,
Uwe
> It looks like the issue is with Postgres's French locale. The error message is not valid UTF-8:
> 
> Raw escaped bytes:
> 
> ```
> \x61\x75\x74\x68\x65\x6e\x74\x69\x66\x69\x63\x61\x74\x69\x6f
> \x6e\x20\x70\x61\x72\x20\x6d\x6f\x74\x20\x64\x65\x20\x70\x61\x73
> \x73\x65\x20\xe9\x63\x68\x6f\x75\xe9\x65\x20\x70\x6f\x75\x72\x20
> \x6c\x27\x75\x74\x69\x6c\x69\x73\x61\x74\x65\x75\x72\x20\x20\xab
> \x20\x6f\x70\x74\x6f\x6d\x61\x74\x61\x20\xbb\x00
> ```
> 
> `String::from_utf8_lossy` decode:
> 
> ```
> authentification par mot de passe �chou�e pour l'utilisateur  � optomata �
> ```
> 
> Note the `�`s, indicating an invalid UTF-8 byte sequence.


per https://learn.microsoft.com/en-us/windows/win32/intl/code-page-identifiers:
1250 	windows-1250 	ANSI Central European; Central European (Windows)
1252 	windows-1252 	ANSI Latin 1; Western European (Windows)


```rust
use encoding_rs;

fn main() {
    let bytes = b"\x61\x75\x74\x68\x65\x6e\x74\x69\x66\x69\x63\x61\x74\x69\x6f\x6e\x20\x70\x61\x72\x20\x6d\x6f\x74\x20\x64\x65\x20\x70\x61\x73\x73\x65\x20\xe9\x63\x68\x6f\x75\xe9\x65\x20\x70\x6f\x75\x72\x20\x6c\x27\x75\x74\x69\x6c\x69\x73\x61\x74\x65\x75\x72\x20\x20\xab\x20\x6f\x70\x74\x6f\x6d\x61\x74\x61\x20\xbb\x00";

    let encodings = vec![
        encoding_rs::WINDOWS_1250,
        encoding_rs::WINDOWS_1252,
    ];
    for enc in &encodings {
        let (cow, encoding_used, had_errors) = enc.decode(bytes);
        println!("encoding_used: {:?}\thad_errors: {:?}\n\tresult: {:?}", encoding_used, had_errors, &cow[..]);
    }
}
```

output:
```console
encoding_used: Encoding { windows-1250 }        had_errors: false
        result: "authentification par mot de passe échouée pour l'utilisateur  « optomata »\0"
encoding_used: Encoding { windows-1252 }        had_errors: false
        result: "authentification par mot de passe échouée pour l'utilisateur  « optomata »\0"
```


Fix patch:
diff --git a/postgres-protocol/src/message/backend.rs b/postgres-protocol/src/message/backend.rs
index c4439b26a..73b169288 100644
--- a/postgres-protocol/src/message/backend.rs
+++ b/postgres-protocol/src/message/backend.rs
@@ -633,7 +633,7 @@ impl<'a> FallibleIterator for ErrorFields<'a> {
         }
 
         let value_end = find_null(self.buf, 0)?;
-        let value = get_str(&self.buf[..value_end])?;
+        let value = &self.buf[..value_end];
         self.buf = &self.buf[value_end + 1..];
 
         Ok(Some(ErrorField { type_, value }))
@@ -642,7 +642,7 @@ impl<'a> FallibleIterator for ErrorFields<'a> {
 
 pub struct ErrorField<'a> {
     type_: u8,
-    value: &'a str,
+    value: &'a [u8],
 }
 
 impl<'a> ErrorField<'a> {
@@ -652,7 +652,13 @@ impl<'a> ErrorField<'a> {
     }
 
     #[inline]
+    #[deprecated(note = "use value_bytes instead", since = "0.6.7")]
     pub fn value(&self) -> &str {
+        str::from_utf8(self.value).expect("error field value contained non-UTF8 bytes")
+    }
+
+    #[inline]
+    pub fn value_bytes(&self) -> &[u8] {
         self.value
     }
 }
diff --git a/tokio-postgres/src/error/mod.rs b/tokio-postgres/src/error/mod.rs
index f1e2644c6..75664d258 100644
--- a/tokio-postgres/src/error/mod.rs
+++ b/tokio-postgres/src/error/mod.rs
@@ -107,14 +107,15 @@ impl DbError {
         let mut routine = None;
 
         while let Some(field) = fields.next()? {
+            let value = String::from_utf8_lossy(field.value_bytes());
             match field.type_() {
-                b'S' => severity = Some(field.value().to_owned()),
-                b'C' => code = Some(SqlState::from_code(field.value())),
-                b'M' => message = Some(field.value().to_owned()),
-                b'D' => detail = Some(field.value().to_owned()),
-                b'H' => hint = Some(field.value().to_owned()),
+                b'S' => severity = Some(value.into_owned()),
+                b'C' => code = Some(SqlState::from_code(&value)),
+                b'M' => message = Some(value.into_owned()),
+                b'D' => detail = Some(value.into_owned()),
+                b'H' => hint = Some(value.into_owned()),
                 b'P' => {
-                    normal_position = Some(field.value().parse::<u32>().map_err(|_| {
+                    normal_position = Some(value.parse::<u32>().map_err(|_| {
                         io::Error::new(
                             io::ErrorKind::InvalidInput,
                             "`P` field did not contain an integer",
@@ -122,32 +123,32 @@ impl DbError {
                     })?);
                 }
                 b'p' => {
-                    internal_position = Some(field.value().parse::<u32>().map_err(|_| {
+                    internal_position = Some(value.parse::<u32>().map_err(|_| {
                         io::Error::new(
                             io::ErrorKind::InvalidInput,
                             "`p` field did not contain an integer",
                         )
                     })?);
                 }
-                b'q' => internal_query = Some(field.value().to_owned()),
-                b'W' => where_ = Some(field.value().to_owned()),
-                b's' => schema = Some(field.value().to_owned()),
-                b't' => table = Some(field.value().to_owned()),
-                b'c' => column = Some(field.value().to_owned()),
-                b'd' => datatype = Some(field.value().to_owned()),
-                b'n' => constraint = Some(field.value().to_owned()),
-                b'F' => file = Some(field.value().to_owned()),
+                b'q' => internal_query = Some(value.into_owned()),
+                b'W' => where_ = Some(value.into_owned()),
+                b's' => schema = Some(value.into_owned()),
+                b't' => table = Some(value.into_owned()),
+                b'c' => column = Some(value.into_owned()),
+                b'd' => datatype = Some(value.into_owned()),
+                b'n' => constraint = Some(value.into_owned()),
+                b'F' => file = Some(value.into_owned()),
                 b'L' => {
-                    line = Some(field.value().parse::<u32>().map_err(|_| {
+                    line = Some(value.parse::<u32>().map_err(|_| {
                         io::Error::new(
                             io::ErrorKind::InvalidInput,
                             "`L` field did not contain an integer",
                         )
                     })?);
                 }
-                b'R' => routine = Some(field.value().to_owned()),
+                b'R' => routine = Some(value.into_owned()),
                 b'V' => {
-                    parsed_severity = Some(Severity::from_str(field.value()).ok_or_else(|| {
+                    parsed_severity = Some(Severity::from_str(&value).ok_or_else(|| {
                         io::Error::new(
                             io::ErrorKind::InvalidInput,
                             "`V` field contained an invalid value",


Test patch: