Double encoding for YAML sessions #524

Closed
shumphrey opened this Issue Jan 14, 2014 · 7 comments

Comments

Projects
None yet
4 participants
@shumphrey
Contributor

shumphrey commented Jan 14, 2014

YAML::Any delegating to YAML::XS and possibly other YAML engines, deal in utf8 octets, not character strings.

    print {$fh} YAML::XS::Dump($data);

That line in Dancer2::Session::YAML prints utf8 octets to $fh. However, in Dancer2::Core::Role::SessionFactory::File, $fh is opened with binmode ":encoding('utf-8')"

This results in a doubly encoded YAML file.

$fh does not have this binmode for reading the file and this produces broken session files.

  • Is it correct for Dancer2::Core::SessionFactory::File to set the file handle encoding?
  • Easy fix is to make Dancer2::Core::YAML set binmode :raw, is this a hack?
@shumphrey

This comment has been minimized.

Show comment Hide comment
@shumphrey

shumphrey Jan 14, 2014

Contributor

Personally, I believe that the correct fix is to not have binmode set in Dancer2::Core::Role::SessionFactory::File, and leave encoding up to the individual Session class.
I can create either that PR or a PR that sets binmode :raw in Dancer2::Session::YAML

Contributor

shumphrey commented Jan 14, 2014

Personally, I believe that the correct fix is to not have binmode set in Dancer2::Core::Role::SessionFactory::File, and leave encoding up to the individual Session class.
I can create either that PR or a PR that sets binmode :raw in Dancer2::Session::YAML

@shumphrey

This comment has been minimized.

Show comment Hide comment
@shumphrey

shumphrey Jan 14, 2014

Contributor

I was about to do a PR that just binmodes $fh, however, YAML::XS Dump returns octects, YAML::Syck Dump returns characters. So YAML::Any fundamentally doesn't work?

Contributor

shumphrey commented Jan 14, 2014

I was about to do a PR that just binmodes $fh, however, YAML::XS Dump returns octects, YAML::Syck Dump returns characters. So YAML::Any fundamentally doesn't work?

@racke

This comment has been minimized.

Show comment Hide comment
@racke

racke Mar 27, 2014

Owner

We (sawyer + racke) just discussed this issue. Using YAML session is not recommended for production, so the performance gain is irrelevant. Thus we agreed on using plain YAML to avoid UTF-8 issues.

And yes, YAML::Any doesn't work in this regard.

Owner

racke commented Mar 27, 2014

We (sawyer + racke) just discussed this issue. Using YAML session is not recommended for production, so the performance gain is irrelevant. Thus we agreed on using plain YAML to avoid UTF-8 issues.

And yes, YAML::Any doesn't work in this regard.

@xsawyerx

This comment has been minimized.

Show comment Hide comment
@xsawyerx

xsawyerx Apr 4, 2014

Owner

Can we have a test for this?

Owner

xsawyerx commented Apr 4, 2014

Can we have a test for this?

@shumphrey

This comment has been minimized.

Show comment Hide comment
@shumphrey

shumphrey Apr 4, 2014

Contributor

yeah, I'll create a test for this today/tomorrow

Contributor

shumphrey commented Apr 4, 2014

yeah, I'll create a test for this today/tomorrow

@shumphrey

This comment has been minimized.

Show comment Hide comment
@shumphrey

shumphrey Apr 5, 2014

Contributor

I knocked up a test I believe demonstrates two issues, one, YAML::Any behaving differently with different implementations, and the other, our serializer double encoding out, and single decoding in. Test file currently lives here: ff3f1be

Contributor

shumphrey commented Apr 5, 2014

I knocked up a test I believe demonstrates two issues, one, YAML::Any behaving differently with different implementations, and the other, our serializer double encoding out, and single decoding in. Test file currently lives here: ff3f1be

@veryrusty

This comment has been minimized.

Show comment Hide comment
@veryrusty

veryrusty Apr 13, 2014

Owner

Resolved by #562.

Owner

veryrusty commented Apr 13, 2014

Resolved by #562.

@veryrusty veryrusty closed this Apr 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment