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