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.

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

Jump to bottom

osc user authentification seems to be broken with last commit #202

Closed
kkeil opened this issue May 28, 2016 · 22 comments · Fixed by #910
Closed

osc user authentification seems to be broken with last commit #202

kkeil opened this issue May 28, 2016 · 22 comments · Fixed by #910

Comments

@kkeil
Copy link
Contributor

kkeil commented May 28, 2016

with current master for example this does not longer work
`osc -c ~/testobsadmin.rc meta prjconf Test10 -F newprjconf
Sending meta data...
Server returned an error: HTTP Error 500: Internal Server Error

Request: https://api.linux-pingi.de/source/Test10/_config
Headers:
content-language: en
accept-ranges: bytes
vary: accept-language,accept-charset
server: Apache
connection: close
cache-control: public
date: Sat, 28 May 2016 10:42:35 GMT
content-type: text/html; charset=utf-8
`
Same command with 0.154.0 works, a quick bisect found
c53a7681 as reason.
@marcus-h
Copy link
Member

marcus-h commented May 29, 2016 via email

@mmohring
Copy link
Contributor

I can reproduce this:

> osc -A https://api.myobsapi.com meta prj -F OBS:Tools:Unstable.xml OBS:Tools:Unstable

returns:

`Sending meta data...
Server returned an error: HTTP Error 500: Internal Server Error

Request: https://api.openlifecyclemanager.com/source/OBS%3ATools%3AUnstable/_meta
Headers:
content-language: en
accept-ranges: bytes
vary: accept-language,accept-charset
server: Apache
connection: close
cache-control: public
date: Sun, 29 May 2016 18:04:51 GMT
content-type: text/html; charset=utf-8
`

while the git commit 8466e49 works.

@mmohring
Copy link
Contributor

I checked with git commit c53a768 , which returns this error.

@mmohring
Copy link
Contributor

Still happens with git commit 502dbaa the current git master.

@marcus-h
Copy link
Member

On 2016-05-29 11:15:45 -0700, Martin Mohring wrote:

I can reproduce this:

> osc -A https://api.myobsapi.com meta prj -F OBS:Tools:Unstable.xml OBS:Tools:Unstable

Can you please run this with the "-d" option in front?

@kkeil
Copy link
Contributor Author

kkeil commented May 30, 2016

osc.txt
This is the output of osc -d. The API server side is 2.6.9.

I can confirm, that it seems to still work with opensuse.org, maybe thats because of the ichain/proxy auth.

The error on the OBS server is (production.log):
f815ad91-4a78-45e3-94ec-60fd87a108a4] [13149:142176.01] Started PUT "/source/Test1/_config" for 10.23.200.0 at 2016-05-30 08:08:08 +0200 [f815ad91-4a78-45e3-94ec-60fd87a108a4] [13149:142176.01] Processing by SourceController#update_project_config as XML [f815ad91-4a78-45e3-94ec-60fd87a108a4] [13149:142176.01] Parameters: {"project"=>"Test1"} [f815ad91-4a78-45e3-94ec-60fd87a108a4] [13149:142176.01] Can't verify CSRF token authenticity [f815ad91-4a78-45e3-94ec-60fd87a108a4] [13149:142176.02] Rendered status.xml.builder (0.3ms) [f815ad91-4a78-45e3-94ec-60fd87a108a4] [13149:142176.02] Completed 401 Unauthorized in 5ms (Views: 1.1ms | ActiveRecord: 0.7ms | Backend: 0.0ms | XML: 0.0ms)

@marcus-h
Copy link
Member

On 2016-05-29 23:43:25 -0700, Karsten Keil wrote:

osc.txt
This is the output of osc -d. The API server side is 2.6.9.

Ah sorry, I was thinking about "-H", but I was writing "-d"...
can you also post the output of "osc -d -H ..." :)

@kkeil
Copy link
Contributor Author

kkeil commented May 30, 2016

osc_d_h_fail.txt
osc_d_h_OK.txt
I did it with osc -d --http-full-debug -c ~/testobsadmin.rc meta prjconf Test1 -F /tmp/new.prjconf since this is an internal test instance. OK is without commit c53a768.

@adrianschroeter
Copy link
Member

@mmohring is the 500 reproducable? In that case I would be interessted in the backtrace of the api. (no client should be able to create this)

@mmohring
Copy link
Contributor

@adrianschroeter Will make a log and put it here. Will try with my local OBS and b.o.o.

@kkeil
Copy link
Contributor Author

kkeil commented May 30, 2016

@adrian yes I see the 500 too, but here is no back trace in the production.log, only what I wrote in the comment above. in the access log I see the 500:
10.23.200.0 - - [30/May/2016:10:59:23 +0200] "PUT /source/Test1/_config HTTP/1.1" 401 112 10.23.200.0 - - [30/May/2016:10:59:23 +0200] "PUT /source/Test1/_config HTTP/1.1" 500 1013

@marcus-h
Copy link
Member

On 2016-05-30 02:10:35 -0700, Karsten Keil wrote:

osc_d_h_fail.txt
osc_d_h_OK.txt
I did it with osc -d --http-full-debug -c ~/testobsadmin.rc meta prjconf Test1 -F /tmp/new.prjconf since this is an internal test instance. OK is without commit c53a768.

Thanks! From a first glance, the logs look good:

osc_d_h_fail.txt:
...
send: <open file '/tmp/osc_metafile.M4gVHU.txt', mode 'rb' at 0x7f80540d85d0>
sendIng a read()able
...

indicates that the correct codepath is used (in this case).
The only difference between the two codepaths is the way how the data
is send to the server (one big "chunk" vs. several "small" "chunks"
(from a high-level perspective))... it seems that the latter might
confuse the apache.

I'll try to reproduce with a local obs instance (as soon as I have it
running again).

@mmohring
Copy link
Contributor

First of all the check with b.o.o.

> rpm -qa | grep osc
osc-0.155.git.1464531661.502dbaa-18.1.noarch

> osc -A https://api.opensuse.org meta prj -F home:MartinMohring:Base:build:arm.xml home:MartinMohring:Base:build:arm
Sending meta data...
Done.

So this works.

@mmohring
Copy link
Contributor

Now the output of:

osc -d -H -A https://api.openlifecyclemanager.com meta prj -F OBS:Server:Unstable.xml OBS:Server:Unstable >& a.log

Attached
a.log.txt

And the server side log also attached
b.log.txt

@mmohring
Copy link
Contributor

I am using openSUSE 13.2 x86_64 as a host for OBS Unstable. Installed git master from git commit 54314f6e74235292a0de902534a1cba5993b1beb , e.g. from Friday. As I said osc git commit c53a768 was when it started not working.

@mmohring
Copy link
Contributor

I will test openSUSE/open-build-service#1840 .

@mmohring
Copy link
Contributor

I have tested openSUSE/open-build-service#1840 , still the same results.

@adrianschroeter
Copy link
Member

1840 is unrelated to this issue

@mmohring
Copy link
Contributor

OK. Anyway, I have tried to accidentially make a osc ci with also an internal 500 error:

> osc ci -m"fixed: make revisioned meta working causing a 500 by current osc git master"

and this resulted also in an error:

c.log.txt

@kkeil
Copy link
Contributor Author

kkeil commented Jun 1, 2016

@marcus-h: I tried this against the last unstable appliance (obs-server qcow2 image), same results.

marcus-h added a commit that referenced this issue Jun 2, 2016
@marcus-h
Revert "Simplify core.http_request"
acbd2c1 
This reverts commit c53a768 (for now!).
It seems to break local obs instances (see issue #202) (this needs
further debugging). Moreover, it breaks the python 3.4 - excerpt
from a travis run:

======================================================================
ERROR: test_added_missing2 (test_commit.TestCommit)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/python/3.4.2/lib/python3.4/urllib/request.py", line 1111, in do_request_
    mv = memoryview(data)
TypeError: memoryview: _io.BufferedReader object does not have the buffer interface

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/travis/build/openSUSE/osc/tests/common.py", line 122, in wrapped_test_method
    test_method(*args)
  File "/home/travis/build/openSUSE/osc/tests/common.py", line 122, in wrapped_test_method
    test_method(*args)
  File "/home/travis/build/openSUSE/osc/tests/common.py", line 122, in wrapped_test_method
    test_method(*args)
  File "/home/travis/build/openSUSE/osc/tests/common.py", line 122, in wrapped_test_method
    test_method(*args)
  File "/home/travis/build/openSUSE/osc/tests/common.py", line 122, in wrapped_test_method
    test_method(*args)
  File "/home/travis/build/openSUSE/osc/tests/test_commit.py", line 290, in test_added_missing2
    p.commit()
  File "/home/travis/build/openSUSE/osc/tests/osc/core.py", line 1471, in commit
    self.put_source_file(filename, tdir)
  File "/home/travis/build/openSUSE/osc/tests/osc/core.py", line 1319, in put_source_file
    http_PUT(u, file = tfilename)
  File "/home/travis/build/openSUSE/osc/tests/osc/core.py", line 3243, in http_PUT
    def http_PUT(*args, **kwargs):    return http_request('PUT', *args, **kwargs)
  File "/home/travis/build/openSUSE/osc/tests/osc/core.py", line 3231, in http_request
    fd = urlopen(req, data=data)
  File "/opt/python/3.4.2/lib/python3.4/urllib/request.py", line 153, in urlopen
    return opener.open(url, data, timeout)
  File "/opt/python/3.4.2/lib/python3.4/urllib/request.py", line 453, in open
    req = meth(req)
  File "/opt/python/3.4.2/lib/python3.4/urllib/request.py", line 1116, in do_request_
    data))
ValueError: Content-Length should be specified for iterable data of type <class '_io.BufferedReader'> <_io.BufferedReader name='/tmp/osc_test571whun4/osctest/added_missing/.osc/_in_commit/bar'>
@marcus-h
Copy link
Member

marcus-h commented Jun 2, 2016

On 2016-06-01 05:13:36 -0700, Karsten Keil wrote:

@marcus-h: I tried this against the last unstable appliance (obs-server qcow2 image), same results.

For now, I reverted the commit again (it also broke the python 3.4
support) (see commit acbd2c1).
I won't close this issue, because it definitely needs further debugging
(I'll have look at it ASAP).

This was referenced Jun 2, 2016
Closed
Open
@abitrolly
Copy link
Contributor

Is this still broken?

@marcus-h marcus-h mentioned this issue Jan 25, 2021
Closed
marcus-h added a commit to marcus-h/osc that referenced this issue Apr 9, 2021
@marcus-h
Support an arbitrary sized file in core.http_request
3f0836c 
The old code only supports a file whose size is less then or equal
to INT_MAX (due to a reasonable(!) limit in M2Crypto). The actual
issue is in core.http_request which mmap(...)s the file, wraps it
into a memoryview/buffer and then passes the memoryview/buffer to
urlopen. Eventually, the whole memoryview/buffer is read into memory
(see m2_PyObject_GetBufferInt). If the file is too large (> INT_MAX),
m2_PyObject_GetBufferInt raises a ValueError (which is perfectly
fine!).
Reading a whole file into memory is completely insane. In order to
avoid this, we now simply pass a file-like object to urlopen (more
precisely, the file-like object is associated with the Request
instance that is passed to urlopen). The advantange is that the
file-like object is processed in chunks of 8192 bytes (see
http.client.HTTPConnection) (that is, only 8192 bytes are read into
memory (instead of the whole file)).

There are two pitfalls when passing a file-like object to urlopen:
* By default, a chunked Transfer-Encoding is applied. It seems that
  some servers (like api.o.o) do like this (PUTing a file with
  a chunked Transfer-Encoding to api.o.o results in status 400). In
  order to avoid a chunked Transfer-Encoding, we explicitly set a
  Content-Length header (we also do this in the non-file case (just
  for the sake of completeness)).
* If the request fails with status 401, it is retried with an
  appropriate Authorization header. When retrying the request, the
  file's offset has to be repositioned to the beginning of the file
  (otherwise, a 0-length body is sent which most likely does not
  match the Content-Length header).

Note: core.http_request's "data" and "file" parameters are now mutually
exclusive because specifying both makes no sense (only one of them
is considered) and it simplifies the implementation a bit.

Fixes: openSUSE#202 ("osc user authentification seems to be broken with last
commit")
Fixes: openSUSE#304 ("osc ci - cannot handle more than 2 GB file uploads")
@marcus-h marcus-h mentioned this issue Apr 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support an arbitrary sized file in core.http_request marcus-h/osc
5 participants
@adrianschroeter @marcus-h @mmohring @kkeil @abitrolly