Skip to content

Use Encoding::BINARY for JOHAB #472

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 7, 2022
Merged

Use Encoding::BINARY for JOHAB #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions ext/pg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,26 +127,6 @@ const char * const (pg_enc_pg2ruby_mapping[][2]) = {
static struct st_table *enc_pg2ruby;


/*
* Look up the JOHAB encoding, creating it as a dummy encoding if it's not
* already defined.
*/
static rb_encoding *
pg_find_or_create_johab(void)
{
static const char * const aliases[] = { "JOHAB", "Windows-1361", "CP1361" };
int enc_index;
size_t i;

for (i = 0; i < sizeof(aliases)/sizeof(aliases[0]); ++i) {
enc_index = rb_enc_find_index(aliases[i]);
if (enc_index > 0) return rb_enc_from_index(enc_index);
}

enc_index = rb_define_dummy_encoding(aliases[0]);
return rb_enc_from_index(enc_index);
}

/*
* Return the given PostgreSQL encoding ID as an rb_encoding.
*
Expand Down Expand Up @@ -187,10 +167,6 @@ pg_get_pg_encname_as_rb_encoding( const char *pg_encname )
return rb_enc_find( pg_enc_pg2ruby_mapping[i][1] );
}

/* JOHAB isn't a builtin encoding, so make up a dummy encoding if it's seen */
if ( strncmp(pg_encname, "JOHAB", 5) == 0 )
return pg_find_or_create_johab();

/* Fallthrough to ASCII-8BIT */
return rb_ascii8bit_encoding();
}
Expand Down
18 changes: 2 additions & 16 deletions spec/pg/connection_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1807,10 +1807,10 @@
expect( @conn.internal_encoding ).to eq( Encoding::ASCII_8BIT )
end

it "the connection should use JOHAB dummy encoding when it's set to JOHAB" do
it "the connection should use the BINARY encoding when it's set to JOHAB" do
@conn.set_client_encoding "JOHAB"
val = @conn.exec("SELECT chr(x'3391'::int)").values[0][0]
expect( val.encoding.name ).to eq( "JOHAB" )
expect( val.encoding ).to eq( Encoding::BINARY )
expect( val.unpack("H*")[0] ).to eq( "dc65" )
end

Expand Down Expand Up @@ -1877,20 +1877,6 @@
expect { @conn.set_client_encoding( nil ) }.to raise_error(TypeError)
end

it "can use an encoding with high index for client encoding" do
# Allocate a lot of encoding indices, so that MRI's ENCODING_INLINE_MAX is exceeded
unless Encoding.name_list.include?("pgtest-0")
256.times do |eidx|
Encoding::UTF_8.replicate("pgtest-#{eidx}")
end
end

# Now allocate the JOHAB encoding with an unusual high index
@conn.set_client_encoding "JOHAB"
val = @conn.exec("SELECT chr(x'3391'::int)").values[0][0]
expect( val.encoding.name ).to eq( "JOHAB" )
end
Copy link
Contributor Author

@eregon eregon Jul 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this test no longer makes sense/can no longer test anything useful since pg does not create encodings dynamically anymore with this PR.
Copy link
Collaborator

@larskanis larskanis Aug 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is actually to ensure

ruby-pg/ext/pg.h

Line 27 in 2821bf7

if ((i) < ENCODING_INLINE_MAX) \
is working correctly. But in practice there's no way to get this condition failing, without the JOHAB encoding. So we can remove the test.

end

describe "respect and convert character encoding of input strings" do
Expand Down