From acbb8ad613cf5e3f3ad276ee32877bffbc7cf547 Mon Sep 17 00:00:00 2001 From: Lukas Rosenthaler Date: Thu, 5 Nov 2020 23:35:01 +0100 Subject: [PATCH] Bugfix: SIPI crashed for some invalid IIIF url's If Sipi received invalid IIIF url's with missing parts, Sipi could crash. The bug is fixed and assorted tests for this behaviour have been added. --- src/SipiHttpServer.cpp | 32 ++++++++++++++++---------------- test/e2e/test_02_server.py | 8 ++++++++ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/SipiHttpServer.cpp b/src/SipiHttpServer.cpp index 105b08f5..2c985193 100644 --- a/src/SipiHttpServer.cpp +++ b/src/SipiHttpServer.cpp @@ -1051,10 +1051,10 @@ namespace Sipi { if (qualform_ok || rotation_ok || size_ok || region_ok) { std::stringstream errmsg; errmsg << "IIIF url not correctly formatted:"; - if (!qualform_ok) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; - if (!rotation_ok) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; - if (!size_ok) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; - if (!region_ok) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; + if (!qualform_ok && (parts.size() > 0)) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; + if (!rotation_ok && (parts.size() > 1)) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; + if (!size_ok && (parts.size() > 2)) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; + if (!region_ok && (parts.size() > 3)) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; send_error(conn_obj, Connection::BAD_REQUEST, errmsg.str()); return; } @@ -1070,10 +1070,10 @@ namespace Sipi { } else { std::stringstream errmsg; errmsg << "IIIF url not correctly formatted:"; - if (!qualform_ok) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; - if (!rotation_ok) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; - if (!size_ok) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; - if (!region_ok) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; + if (!qualform_ok && (parts.size() > 0)) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; + if (!rotation_ok && (parts.size() > 1)) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; + if (!size_ok && (parts.size() > 2)) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; + if (!region_ok && (parts.size() > 3)) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; send_error(conn_obj, Connection::BAD_REQUEST, errmsg.str()); return; } @@ -1109,10 +1109,10 @@ namespace Sipi { if (qualform_ok || rotation_ok || size_ok || region_ok) { std::stringstream errmsg; errmsg << "IIIF url not correctly formatted:"; - if (!qualform_ok) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; - if (!rotation_ok) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; - if (!size_ok) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; - if (!region_ok) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; + if (!qualform_ok && (parts.size() > 0)) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; + if (!rotation_ok && (parts.size() > 1)) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; + if (!size_ok && (parts.size() > 2)) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; + if (!region_ok && (parts.size() > 3)) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; send_error(conn_obj, Connection::BAD_REQUEST, errmsg.str()); return; } @@ -1128,10 +1128,10 @@ namespace Sipi { } else { std::stringstream errmsg; errmsg << "IIIF url not correctly formatted:"; - if (!qualform_ok) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; - if (!rotation_ok) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; - if (!size_ok) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; - if (!region_ok) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; + if (!qualform_ok && (parts.size() > 0)) errmsg << " Error in quality: \"" << parts[parts.size() - 1] << "\"!"; + if (!rotation_ok && (parts.size() > 1)) errmsg << " Error in rotation: \"" << parts[parts.size() - 2] << "\"!"; + if (!size_ok && (parts.size() > 2)) errmsg << " Error in size: \"" << parts[parts.size() - 3] << "\"!"; + if (!region_ok && (parts.size() > 3)) errmsg << " Error in region: \"" << parts[parts.size() - 4] << "\"!"; send_error(conn_obj, Connection::BAD_REQUEST, errmsg.str()); return; } diff --git a/test/e2e/test_02_server.py b/test/e2e/test_02_server.py index 9da3eea9..bb361211 100644 --- a/test/e2e/test_02_server.py +++ b/test/e2e/test_02_server.py @@ -89,6 +89,14 @@ def test_deny(self, manager): """return 401 Unauthorized if the user does not have permission to see the image""" manager.expect_status_code("/knora/DenyLeaves.jpg/full/max/0/default.jpg", 401) + def test_iiifurl_parsing(self, manager): + """Return 400 for invalid IIIF URL's""" + manager.expect_status_code("/unit//lena512.jp2", 400) + manager.expect_status_code("/unit/lena512.jp2/max/0/default.jpg", 400) + manager.expect_status_code("/unit/lena512.jp2/full/max/default.jpg", 400) + manager.expect_status_code("/unit/lena512.jp2/full/max/!/default.jpg", 400) + manager.expect_status_code("/unit/lena512.jp2/full/max/0/jpg", 400) + def test_read_write(self, manager): """read an image file, convert it to JPEG2000 and write it"""