Skip to content
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.

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

Reverse operation for strings_to_byte #31

Closed
xguerin opened this issue Jan 4, 2018 · 8 comments
Closed

Reverse operation for strings_to_byte #31

xguerin opened this issue Jan 4, 2018 · 8 comments

Comments

@xguerin
Copy link

xguerin commented Jan 4, 2018

Hello there,

I am trying to use Markup with Cohttp_lwt to do some HTML rewriting. I followed the example to get it to parse, but I am running into some issues to write a Markup stream into a Cohttp body due to the lack of reciprocity of the function strings_to_byte.

Here is what I am trying to to:

  let body = body
             |> Body.to_stream
             |> Markup_lwt.lwt_stream
             |> Markup.strings_to_bytes
             |> Markup.parse_html
             |> Markup.signals
             |> Markup.write_html
             |> Markup_lwt.to_lwt_stream
             |> Body.of_stream

Unfortunately, the last operation is not compatible because to_lwt_stream returns a (char, 'a) stream while of_stream expects a (string, 'a) stream. I could not find a reciprocal version of strings_to_byte to hopefully do the job. Am I looking at the right place ?

Thanks,

@xguerin
Copy link
Author

xguerin commented Jan 4, 2018

Actually, reading the code it looks like Htlml_writer and Xml_writer already return string streams, and write_html and write_xml transform that stream into a char stream.

EDIT

I locally added write_* functions that return a (string, 'a) stream and ended up with interesting EPIPE errors. As a workaround, I am passing through a string instead of a stream and it works.

@aantron
Copy link
Owner

aantron commented Jan 5, 2018

Thanks, I'll take a look. This seems like it would be helped by addressing #10.

@aantron
Copy link
Owner

aantron commented Jan 5, 2018

@xguerin, do you know what was causing the EPIPE signals?

@xguerin
Copy link
Author

xguerin commented Jan 5, 2018

Yes, #10 would definitely be the answer (this is basically what I did locally). Regarding EPIPE, I am still investigating. But I think there may be an underlying problem. If I don't use streams but pure strings, at some points I would get truncated/incomplete data (the most obvious consequence of that is that the web browser hangs on loading the page). However, if I skip markup's parsing/writing phase, all is well (so it's not a Lwt/Cohttp issue).

@aantron
Copy link
Owner

aantron commented Jan 5, 2018

Would you want to submit your patch as a PR? Otherwise let me know, I'll do it :)

I'm not sure still if you mean the second part is a bug in Markup.ml, nor sure myself if it would be. My guess would be no, because Markup.ml doesn't do its own I/O, but it could be triggering some kind of I/O pattern that it shouldn't trigger.

@xguerin
Copy link
Author

xguerin commented Jan 5, 2018

Yes definitely. Re: the other issue, I'll look into it and let you know.

@xguerin
Copy link
Author

xguerin commented Jan 6, 2018

More info on the choke/EPIPE: I am not able to reproduce it when reading from file and writing to file or reading from a socket (Cohttp_lwt_unix.Client) and writing to file. So it looks like there is something amok with the server socket on the return path. More to come.

@aantron
Copy link
Owner

aantron commented Jan 7, 2018

Thanks for continuing to look into it.

@aantron aantron added this to the 0.8.2 milestone Aug 17, 2019
@aantron aantron removed this from the 0.8.2 milestone Oct 22, 2020
@xguerin xguerin closed this as not planned Won't fix, can't repro, duplicate, stale Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants