<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Oct 15, 2018 at 6:21 PM Richard W.M. Jones <<a href="mailto:rjones@redhat.com">rjones@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, Oct 09, 2018 at 02:28:10PM +0300, Nir Soffer wrote:<br>
> > +# Create a background thread running a web server which is<br>
> > +# simulating the imageio server.<br>
> ><br>
> <br>
> This functionality should be separated from the fake SDK module, since it is<br>
> not part of the SDK, and may be replaced by real imageio server later.<br>
<br>
Well possibly, but it was very convenient to put it in the class here,<br>
and this test is meant for running completely standalone without any<br>
other service available.<br>
<br>
> > +class RequestHandler(BaseHTTPRequestHandler):<br>
> ><br>
> <br>
> This request handler is using HTTP/1.0, and will close the connection after<br>
> every request. This is not a good implementation of the imageio server, and<br>
> also<br>
> hides bugs in this code.<br>
> <br>
> Should be fixed by adding:<br>
> <br>
>     protocol_version = "HTTP/1.1"<br>
<br>
I tried the attached patch, but for some reason it just hangs.<br></blockquote><div><br></div><div>The issue is probably missing "content-length: 0" header in the</div><div>response. If we don't close the connection and don't send </div><div>content-length the client cannot do much but wait :-)</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> > +    def do_OPTIONS(self):<br>
> <br>
> +        self.send_response(200)<br>
> > +        self.send_header("Content-type", "application/json;<br>
> > charset=UTF-8")<br>
> > +        self.end_headers()<br>
> > +        # Advertize only zero support.<br>
> > +        self.wfile.write(b'''{ "features": [ "zero" ] }''')<br>
> > +<br>
> > +    # eg. zero request.  Just ignore it.<br>
> > +    def do_PATCH(self):<br>
> ><br>
> <br>
> This must read content-length bytes from self.rfile.<br>
> <br>
> <br>
> > +        self.send_response(200)<br>
> > +        self.end_headers()<br>
> > +<br>
> > +    # Flush request.  Ignore it.<br>
> > +    def do_PUT(self):<br>
> ><br>
> <br>
> This must read content-length bytes from self.rfile.<br>
> <br>
> Both bugs are hidden by the fact that this server close the connection at<br>
> the end<br>
> of the request.<br>
<br>
Also this is covered by the attached patch.<br>
<br>
> > +        self.send_response(200)<br>
> > +        self.end_headers()<br>
> > +<br>
> > +def server():<br>
> > +    server_address = ("", imageio_port)<br>
> > +    httpd = HTTPServer(server_address, RequestHandler)<br>
> > +    httpd.serve_forever()<br>
> ><br>
><br>
> Using HTTP instead of HTTPS is not a good idea. We are not testing<br>
> the same code on the client side.<br>
><br>
> It is easy to run HTTPS server using pre-created certificate, if we<br>
> disable certificate verification on the client side<br>
> (e.g. --insecure).<br>
<br>
This one doesn't seem to be as simple to fix.  However point taken, so<br>
I added a note in the code.<br>
<br>
Rich.<br>
<br>
-- <br>
Richard Jones, Virtualization Group, Red Hat <a href="http://people.redhat.com/~rjones" rel="noreferrer" target="_blank">http://people.redhat.com/~rjones</a><br>
Read my programming and virtualization blog: <a href="http://rwmj.wordpress.com" rel="noreferrer" target="_blank">http://rwmj.wordpress.com</a><br>
virt-builder quickly builds VMs from scratch<br>
<a href="http://libguestfs.org/virt-builder.1.html" rel="noreferrer" target="_blank">http://libguestfs.org/virt-builder.1.html</a><br>
</blockquote></div></div>