Project

General

Profile

Bug #34417

Register URLs with and without a slash to avoid a redirect

Added by Anders Kristiansen over 2 years ago. Updated about 2 years ago.

Status:
Done
Priority:
No priority
Assignee:
William Grzybowski
Category:
Middleware
Target version:
Seen in:
Severity:
Low
Reason for Closing:
Reason for Blocked:
Needs QA:
No
Needs Doc:
No
Needs Merging:
No
Needs Automation:
No
Support Suite Ticket:
n/a
Hardware Configuration:
ChangeLog Required:
No
Tags:

Description

On 2.0 API the documentation says to POST to eg. /api/v2.0/vm/id/4/start to start the VM with id 4, but I get an internal server error saying "IndexError: list index out of range". I got the id from GET /api/v2.0/vm, so it should be correct.

I am using FreeNAS-11.1-U4.

The full error response is listed below and I'm using ARC (Advanced REST Client - Chrome addon) to test it.

<h1>500 Internal Server Error</h1><br><h2>Traceback:</h2>

Traceback (most recent call last):
  File &quot;/usr/local/lib/python3.6/site-packages/aiohttp/web_protocol.py&quot;, line 421, in start
    resp = yield from self._request_handler(request)
  File &quot;/usr/local/lib/python3.6/site-packages/aiohttp/web.py&quot;, line 303, in _handle
    resp = yield from handler(request)
  File &quot;/usr/local/lib/python3.6/asyncio/coroutines.py&quot;, line 109, in __next__
    return self.gen.send(None)
  File &quot;/usr/local/lib/python3.6/site-packages/middlewared/restful.py&quot;, line 359, in on_method
    return await do(method, req, resp, *args, **kwargs)
  File &quot;/usr/local/lib/python3.6/asyncio/coroutines.py&quot;, line 109, in __next__
    return self.gen.send(None)
  File &quot;/usr/local/lib/python3.6/site-packages/middlewared/restful.py&quot;, line 484, in do
    result = await self.middleware.call(methodname, *method_args)
  File &quot;/usr/local/lib/python3.6/asyncio/coroutines.py&quot;, line 109, in __next__
    return self.gen.send(None)
  File &quot;/usr/local/lib/python3.6/site-packages/middlewared/main.py&quot;, line 912, in call
    return await self._call(name, serviceobj, methodobj, params)
  File &quot;/usr/local/lib/python3.6/asyncio/coroutines.py&quot;, line 109, in __next__
    return self.gen.send(None)
  File &quot;/usr/local/lib/python3.6/site-packages/middlewared/main.py&quot;, line 876, in _call
    return await methodobj(*args)
  File &quot;/usr/local/lib/python3.6/asyncio/coroutines.py&quot;, line 109, in __next__
    return self.gen.send(None)
  File &quot;/usr/local/lib/python3.6/site-packages/middlewared/schema.py&quot;, line 490, in nf
    args, kwargs = clean_and_validate_args(args, kwargs)
  File &quot;/usr/local/lib/python3.6/site-packages/middlewared/schema.py&quot;, line 445, in clean_and_validate_args
    attr = nf.accepts[i]
IndexError: list index out of range

Bonus bug: /status doesn't seem to work either (why is it POST and not GET?). I don't need that one for now, but feel free to look into it if anyone wants :)

traceback.txt (2.83 KB) traceback.txt Zackary Welch, 06/26/2018 03:56 PM

Associated revisions

Revision 95c2d6cb (diff)
Added by William Grzybowski over 2 years ago

feat(api): register URLs with and without slash, avoiding a redirect Ticket: #34417

Revision d21089ee (diff)
Added by William Grzybowski over 2 years ago

feat(api): register URLs with and without slash, avoiding a redirect Ticket: #34417

Revision 2fa945fb (diff)
Added by William Grzybowski about 2 years ago

feat(api): register URLs with and without slash, avoiding a redirect Ticket: #34417

History

#1 Updated by Dru Lavigne over 2 years ago

  • Assignee changed from Release Council to William Grzybowski

#2 Updated by William Grzybowski over 2 years ago

  • Status changed from Unscreened to Not Started
  • Target version changed from Backlog to 11.2-RC2
  • Needs Doc changed from Yes to No
  • Needs Merging changed from Yes to No

#3 Updated by William Grzybowski over 2 years ago

  • Status changed from Not Started to Blocked
  • Reason for Blocked set to Waiting for feedback

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

#4 Updated by Anders Kristiansen over 2 years ago

William Grzybowski wrote:

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

I have just tried it on FreeNAS-11.2-MASTER-201806080448 and I get a "308: Permanent Redirect" on all requests related to /vm or /vm/id/...

I can get the VMs on 1.0 API just fine with /api/v1.0/vm/vm

#5 Updated by Dru Lavigne over 2 years ago

  • Status changed from Blocked to Unscreened
  • Reason for Blocked deleted (Waiting for feedback)

#6 Updated by William Grzybowski over 2 years ago

  • Status changed from Unscreened to Blocked
  • Reason for Blocked set to Waiting for feedback

Anders Kristiansen wrote:

William Grzybowski wrote:

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

I have just tried it on FreeNAS-11.2-MASTER-201806080448 and I get a "308: Permanent Redirect" on all requests related to /vm or /vm/id/...

I can get the VMs on 1.0 API just fine with /api/v1.0/vm/vm

It is a 308 Redirect if you read it correctly. You just gotta follow it. -L if you are using curl.

#7 Updated by Anders Kristiansen over 2 years ago

William Grzybowski wrote:

Anders Kristiansen wrote:

William Grzybowski wrote:

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

I have just tried it on FreeNAS-11.2-MASTER-201806080448 and I get a "308: Permanent Redirect" on all requests related to /vm or /vm/id/...

I can get the VMs on 1.0 API just fine with /api/v1.0/vm/vm

It is a 308 Redirect if you read it correctly. You just gotta follow it. -L if you are using curl.

Hmm this is really weird.

On 11.1: /api/v2.0/vm/id/4/start works as long as i don't send any data (not even empty body). If I do, I get internal server error.
On 11.2: /api/v2.0/vm/id/4/start redirects to /api/v2.0/vm/id/4/start/ (yes, only a / appended). It appears to work, when appending a / to the url and not posting any body.

However, when appending a / to the url on 11.1 I get a 404 Not found.

It really should work both with and without the / suffix on both versions (or at least the same). I would also argue that the body should be ignored and thus posting an empty body or even {} as application/json should work just as well.

#8 Updated by William Grzybowski over 2 years ago

Anders Kristiansen wrote:

William Grzybowski wrote:

Anders Kristiansen wrote:

William Grzybowski wrote:

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

I have just tried it on FreeNAS-11.2-MASTER-201806080448 and I get a "308: Permanent Redirect" on all requests related to /vm or /vm/id/...

I can get the VMs on 1.0 API just fine with /api/v1.0/vm/vm

It is a 308 Redirect if you read it correctly. You just gotta follow it. -L if you are using curl.

Hmm this is really weird.

On 11.1: /api/v2.0/vm/id/4/start works as long as i don't send any data (not even empty body). If I do, I get internal server error.
On 11.2: /api/v2.0/vm/id/4/start redirects to /api/v2.0/vm/id/4/start/ (yes, only a / appended). It appears to work, when appending a / to the url and not posting any body.

However, when appending a / to the url on 11.1 I get a 404 Not found.

It really should work both with and without the / suffix on both versions (or at least the same).

We cant change the past. It works with both on nightlies, as long as you have redirect working.

I would also argue that the body should be ignored and thus posting an empty body or even {} as application/json should work just as well.

It works POSTing with empty body

curl -L -u root:freenas  http://192.168.122.147/api/v2.0/vm/id/1/start/ -X POST 
false
curl -L -u root:freenas  http://192.168.122.147/api/v2.0/vm/id/1/start/ -X POST -d ''
false        

Is there anything still actionable here?

#9 Updated by Anders Kristiansen over 2 years ago

William Grzybowski wrote:

Anders Kristiansen wrote:

William Grzybowski wrote:

Anders Kristiansen wrote:

William Grzybowski wrote:

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

I have just tried it on FreeNAS-11.2-MASTER-201806080448 and I get a "308: Permanent Redirect" on all requests related to /vm or /vm/id/...

I can get the VMs on 1.0 API just fine with /api/v1.0/vm/vm

It is a 308 Redirect if you read it correctly. You just gotta follow it. -L if you are using curl.

Hmm this is really weird.

On 11.1: /api/v2.0/vm/id/4/start works as long as i don't send any data (not even empty body). If I do, I get internal server error.
On 11.2: /api/v2.0/vm/id/4/start redirects to /api/v2.0/vm/id/4/start/ (yes, only a / appended). It appears to work, when appending a / to the url and not posting any body.

However, when appending a / to the url on 11.1 I get a 404 Not found.

It really should work both with and without the / suffix on both versions (or at least the same).

We cant change the past. It works with both on nightlies, as long as you have redirect working.

I would also argue that the body should be ignored and thus posting an empty body or even {} as application/json should work just as well.

It works POSTing with empty body

[...]

Is there anything still actionable here?

I think an API should "just work" - this doesn't tbh. ARC (Advanced REST Client - Chrome addon) won't work with either version because of the body problem (I'm not sure exactly what the problem is). Android HttpURLConnection won't work by default, as it doesn't follow redirects, so you need to search for how to handle it when it doesn't work. The code shown below doesn't work because of the body problem (Android/Java). Just the fact that there is a difference between appending a / and not, is strange to me (a redirect does also cost more in performance on both ends).

HOWEVER: I do understand your position. It is possible to make it work and I have solved it in my app by creating a custom function for POSTing to 2.0 that omits any body and handles the redirects manually. If you want to close it, its fine by me.

HttpURLConnection conn = (HttpURLConnection) url.openConnection();
conn.setReadTimeout(timeout);
conn.setConnectTimeout(timeout);
conn.setRequestProperty("Authorization", "Basic " + current.auth);
conn.setRequestProperty("Content-Type", "application/json");
conn.setRequestMethod("POST");
conn.setDoInput(true);
conn.setDoOutput(true);

JSONObject root = new JSONObject();

String str = root.toString();
byte[] outputBytes = str.getBytes("UTF-8");
OutputStream os = conn.getOutputStream();
os.write(outputBytes);

int responseCode = conn.getResponseCode();

#10 Updated by William Grzybowski over 2 years ago

Anders Kristiansen wrote:

William Grzybowski wrote:

Anders Kristiansen wrote:

William Grzybowski wrote:

Anders Kristiansen wrote:

William Grzybowski wrote:

I believe this is fixed in 11.2 Nightlies. Any chance you could try that temporarily?

I have just tried it on FreeNAS-11.2-MASTER-201806080448 and I get a "308: Permanent Redirect" on all requests related to /vm or /vm/id/...

I can get the VMs on 1.0 API just fine with /api/v1.0/vm/vm

It is a 308 Redirect if you read it correctly. You just gotta follow it. -L if you are using curl.

Hmm this is really weird.

On 11.1: /api/v2.0/vm/id/4/start works as long as i don't send any data (not even empty body). If I do, I get internal server error.
On 11.2: /api/v2.0/vm/id/4/start redirects to /api/v2.0/vm/id/4/start/ (yes, only a / appended). It appears to work, when appending a / to the url and not posting any body.

However, when appending a / to the url on 11.1 I get a 404 Not found.

It really should work both with and without the / suffix on both versions (or at least the same).

We cant change the past. It works with both on nightlies, as long as you have redirect working.

I would also argue that the body should be ignored and thus posting an empty body or even {} as application/json should work just as well.

It works POSTing with empty body

[...]

Is there anything still actionable here?

I think an API should "just work" - this doesn't tbh. ARC (Advanced REST Client - Chrome addon) won't work with either version because of the body problem (I'm not sure exactly what the problem is). Android HttpURLConnection won't work by default, as it doesn't follow redirects, so you need to search for how to handle it when it doesn't work. The code shown below doesn't work because of the body problem (Android/Java). Just the fact that there is a difference between appending a / and not, is strange to me (a redirect does also cost more in performance on both ends).

HOWEVER: I do understand your position. It is possible to make it work and I have solved it in my app by creating a custom function for POSTing to 2.0 that omits any body and handles the redirects manually. If you want to close it, its fine by me.

HttpURLConnection conn = (HttpURLConnection) url.openConnection();
conn.setReadTimeout(timeout);
conn.setConnectTimeout(timeout);
conn.setRequestProperty("Authorization", "Basic " + current.auth);
conn.setRequestProperty("Content-Type", "application/json");
conn.setRequestMethod("POST");
conn.setDoInput(true);
conn.setDoOutput(true);

JSONObject root = new JSONObject();

String str = root.toString();
byte[] outputBytes = str.getBytes("UTF-8");
OutputStream os = conn.getOutputStream();
os.write(outputBytes);

int responseCode = conn.getResponseCode();

You seem to be sending a body here ("{}"), thats not the same as empty. Empty ("") works just finr as far as I am aware.

#11 Updated by William Grzybowski over 2 years ago

  • Status changed from Blocked to In Progress
  • Target version changed from 11.2-RC2 to 11.2-BETA1
  • Severity changed from New to Low

I have changed it to not do a redirect, however i dont think there is anything i can do about empty json object ({}) at this time.

https://github.com/freenas/freenas/pull/1389

#12 Updated by William Grzybowski over 2 years ago

  • Status changed from In Progress to Ready for Testing
  • Reason for Blocked deleted (Waiting for feedback)

#13 Updated by Dru Lavigne over 2 years ago

  • Subject changed from Error when trying to start VM using 2.0 REST API to Register URLs with and without a slash to avoid a redirect

#14 Avatar?id=55038&size=24x24 Updated by Zackary Welch about 2 years ago

I am currently testing this and I am getting a "list index out of range" error whenever I run /vm/id/{id}/start, regardless of a slash. I will try again with a new build before I mark this as failed.

#15 Updated by William Grzybowski about 2 years ago

Zackary Welch wrote:

I am currently testing this and I am getting a "list index out of range" error whenever I run /vm/id/{id}/start, regardless of a slash. I will try again with a new build before I mark this as failed.

Thats expected if the id does not exist.

#16 Avatar?id=55038&size=24x24 Updated by Zackary Welch about 2 years ago

This is occurring even if the id exists. I'm using the same id I get from a GET /vm request.

#17 Updated by William Grzybowski about 2 years ago

Zackary Welch wrote:

This is occurring even if the id exists. I'm using the same id I get from a GET /vm request.

Can you share the traceback even though its unrelated to this ticket?

#19 Avatar?id=55038&size=24x24 Updated by Zackary Welch about 2 years ago

  • Private changed from No to Yes

#20 Avatar?id=55038&size=24x24 Updated by Zackary Welch about 2 years ago

  • Private changed from Yes to No

#21 Updated by Dru Lavigne about 2 years ago

  • Status changed from Ready for Testing to In Progress

#22 Updated by William Grzybowski about 2 years ago

  • Status changed from In Progress to Ready for Testing

#24 Avatar?id=55038&size=24x24 Updated by Zackary Welch about 2 years ago

  • Status changed from Ready for Testing to Done
  • Needs QA changed from Yes to No

I'm getting no redirect issue with a slash or index out of range when running this API call. I was able to get it working by avoiding the "-d '{}'". However, when I call vm/id/2/start, all I get is "true" and the VM does not start. This might be a separate issue.

Also available in: Atom PDF