LearningLocker / learninglocker

Learning Locker - The Open Source Learning Record Store. Started in 2014.
https://learningpool.com/solutions/learning-record-store-learning-locker/learning-locker-community-overview/
GNU General Public License v3.0
554 stars 276 forks source link

Return HTTP code 204 with body #251

Closed illerrr closed 10 years ago

illerrr commented 10 years ago

LearningLocker return not empty message body when requested not existent activities/state. According to http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html, LL should not do so.

The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields.

For example, to play:

HTTP request

GET /data/xAPI/activities/state?stateId=XXX&agent=YYY&activityId=ZZZ HTTP/1.1
Content-Type: */*
Accept: application/json
Authorization: Basic ###
User-Agent: Apache CXF 2.7.3
Cache-Control: no-cache
Pragma: no-cache
Host: elearning.otr.ru:8080
Connection: keep-alive

LearningLocker http response

HTTP/1.1 204 No Content
Server: ###
Date: Wed, 11 Jun 2014 07:10:14 GMT
Content-Type: text/html
Connection: keep-alive
Keep-Alive: timeout=60
X-Powered-By: PHP/5.5.12
Cache-Control: max-age=86400
X-Frame-Options: SAMEORIGIN
Set-Cookie: ###
Expires: Thu, 12 Jun 2014 07:10:14 GMT

{"error":{"type":"Symfony\\Component\\HttpKernel\\Exception\\HttpException","message":"","file":"\/data01\/learninglocker-1.0.3\/vendor\/laravel\/framework\/src\/Illuminate\/Foundation\/Application.php","line":875}}

Error trace in log

production.ERROR: exception 'Symfony\Component\HttpKernel\Exception\HttpException' in /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Foundation/Application.php:875
Stack trace:
#0 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(208): Illuminate\Foundation\Application->abort(204)
#1 /data01/learninglocker.devel/app/controllers/xapi/DocumentController.php(129): Illuminate\Support\Facades\Facade::__callStatic('abort', Array)
#2 /data01/learninglocker.devel/app/controllers/xapi/DocumentController.php(129): Illuminate\Support\Facades\App::abort(204)
#3 /data01/learninglocker.devel/app/controllers/xapi/StateController.php(68): Controllers\xAPI\DocumentController->documentResponse(Array)
#4 /data01/learninglocker.devel/app/controllers/xapi/DocumentController.php(37): Controllers\xAPI\StateController->get()
#5 [internal function]: Controllers\xAPI\DocumentController->index()
#6 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/Controller.php(231): call_user_func_array(Array, Array)
#7 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(93): Illuminate\Routing\Controller->callAction('index', Array)
#8 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(62): Illuminate\Routing\ControllerDispatcher->call(Object(Controllers\xAPI\StateController), Object(Illuminate\Routing\Route), 'inde
x')
#9 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(934): Illuminate\Routing\ControllerDispatcher->dispatch(Object(Illuminate\Routing\Route), Object(Illuminate\Http\Request), 'Controllers\\xAA...',
'index')
#10 [internal function]: Illuminate\Routing\Router->Illuminate\Routing\{closure}()
#11 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/Route.php(105): call_user_func_array(Object(Closure), Array)
#12 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(1000): Illuminate\Routing\Route->run(Object(Illuminate\Http\Request))
#13 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Routing/Router.php(968): Illuminate\Routing\Router->dispatchToRoute(Object(Illuminate\Http\Request))
#14 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(738): Illuminate\Routing\Router->dispatch(Object(Illuminate\Http\Request))
#15 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(708): Illuminate\Foundation\Application->dispatch(Object(Illuminate\Http\Request))
#16 /data01/learninglocker.devel/vendor/barryvdh/laravel-cors/src/Barryvdh/Cors/CorsMiddleware.php(33): Illuminate\Foundation\Application->handle(Object(Illuminate\Http\Request), 1, true)
#17 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Http/FrameGuard.php(38): Barryvdh\Cors\CorsMiddleware->handle(Object(Illuminate\Http\Request), 1, true)
#18 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Session/Middleware.php(72): Illuminate\Http\FrameGuard->handle(Object(Illuminate\Http\Request), 1, true)
#19 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Cookie/Queue.php(47): Illuminate\Session\Middleware->handle(Object(Illuminate\Http\Request), 1, true)
#20 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Cookie/Guard.php(51): Illuminate\Cookie\Queue->handle(Object(Illuminate\Http\Request), 1, true)
#21 /data01/learninglocker.devel/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Illuminate\Cookie\Guard->handle(Object(Illuminate\Http\Request), 1, true)
#22 /data01/learninglocker.devel/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(606): Stack\StackedHttpKernel->handle(Object(Illuminate\Http\Request))
#23 /data01/learninglocker.devel/public/index.php(49): Illuminate\Foundation\Application->run()
#24 {main}
ht2 commented 10 years ago

I believe this is caused here. The Illuminate\Foundation\Application class is then catching this and throwing an HttpException.

I believe the simplest fix would be to make the controller return the following instead: return Response::make("", 204);

illerrr commented 10 years ago

Yes, we did the same thing as a temporary patch. Everything working good.

ht2 commented 10 years ago

OK great, I will submit a change

ht2 commented 10 years ago

OK, all merged into master, this will be included as part of 1.0.4's release