add URI encode/decode API Tinygltf is able to write files defined by a URI, so it needs to be able to decode it. Since it may also modify the path where the image is saved it may need to re-encode the URI too. This patch provides an API to set URI encoding and decoding callbacks and exposes the default decode method. Uses the existing dlib::urldecode as a decode default. The encode callback is left null, matching existing behaviour. Updates the WriteImageDataFunction signature to include tinygltf::URICallbacks. Decodes the image and buffer uris before using them as a filename. If the encode callback is set, encodes the written image location in the default WriteImageDataFunction and encodes generated buffer locations when writing .bin files. Adds a save+load step to the test image-uri-spaces to verify uri encoding.
diff --git a/tests/tester.cc b/tests/tester.cc index 00197d6..b1be601 100644 --- a/tests/tester.cc +++ b/tests/tester.cc
@@ -392,27 +392,105 @@ TEST_CASE("image-uri-spaces", "[issue-236]") { - tinygltf::Model model; tinygltf::TinyGLTF ctx; std::string err; std::string warn; // Test image file with single spaces. - bool ret = ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/CubeImageUriSpaces/CubeImageUriSpaces.gltf"); - if (!err.empty()) { - std::cerr << err << std::endl; - } + { + tinygltf::Model model; + bool ret = ctx.LoadASCIIFromFile( + &model, &err, &warn, + "../models/CubeImageUriSpaces/CubeImageUriSpaces.gltf"); + if (!warn.empty()) { + std::cerr << warn << std::endl; + } + if (!err.empty()) { + std::cerr << err << std::endl; + } - REQUIRE(true == ret); + REQUIRE(true == ret); + REQUIRE(warn.empty()); + REQUIRE(err.empty()); + REQUIRE(model.images.size() == 1); + REQUIRE(model.images[0].uri.find(' ') != std::string::npos); + } // Test image file with a beginning space, trailing space, and greater than // one consecutive spaces. - ret = ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/CubeImageUriSpaces/CubeImageUriMultipleSpaces.gltf"); + tinygltf::Model model; + bool ret = ctx.LoadASCIIFromFile( + &model, &err, &warn, + "../models/CubeImageUriSpaces/CubeImageUriMultipleSpaces.gltf"); + if (!warn.empty()) { + std::cerr << warn << std::endl; + } if (!err.empty()) { std::cerr << err << std::endl; } REQUIRE(true == ret); + REQUIRE(warn.empty()); + REQUIRE(err.empty()); + REQUIRE(model.images.size() == 1); + REQUIRE(model.images[0].uri.size() > 1); + REQUIRE(model.images[0].uri[0] == ' '); + + // Test the URI encoding API by saving and re-load the file, without embedding + // the image. + // TODO(syoyo): create temp directory. + { + // Encoder that only replaces spaces with "%20". + auto uriencode = [](const std::string &in_uri, + const std::string &object_type, std::string *out_uri, + void *user_data) -> bool { + (void)user_data; + bool imageOrBuffer = object_type == "image" || object_type == "buffer"; + REQUIRE(true == imageOrBuffer); + *out_uri = {}; + for (char c : in_uri) { + if (c == ' ') + *out_uri += "%20"; + else + *out_uri += c; + } + return true; + }; + + // Remove the buffer URI, so a new one is generated based on the gltf + // filename and then encoded with the above callback. + model.buffers[0].uri.clear(); + + tinygltf::URICallbacks uri_cb{uriencode, tinygltf::URIDecode, nullptr}; + ctx.SetURICallbacks(uri_cb); + ret = ctx.WriteGltfSceneToFile(&model, " issue-236.gltf", false, false); + REQUIRE(true == ret); + + // read back serialized glTF + tinygltf::Model saved; + bool ret = ctx.LoadASCIIFromFile(&saved, &err, &warn, " issue-236.gltf"); + if (!err.empty()) { + std::cerr << err << std::endl; + } + REQUIRE(true == ret); + REQUIRE(err.empty()); + REQUIRE(!warn.empty()); // relative image path won't exist in tests/ + REQUIRE(saved.images.size() == model.images.size()); + + // The image uri in CubeImageUriMultipleSpaces.gltf is not encoded and + // should be different after encoding spaces with %20. + REQUIRE(model.images[0].uri != saved.images[0].uri); + + // Verify the image path remains the same after uri decoding + std::string image_uri, image_uri_saved; + (void)tinygltf::URIDecode(model.images[0].uri, &image_uri, nullptr); + (void)tinygltf::URIDecode(saved.images[0].uri, &image_uri_saved, nullptr); + REQUIRE(image_uri == image_uri_saved); + + // Verify the buffer's generated and encoded URI + REQUIRE(saved.buffers.size() == model.buffers.size()); + REQUIRE(saved.buffers[0].uri == "%20issue-236.bin"); + } } TEST_CASE("serialize-empty-material", "[issue-294]") { @@ -583,7 +661,11 @@ auto writer = [](const std::string *basepath, const std::string *filename, const tinygltf::Image *image, bool embedImages, - std::string *out_uri, void *user_pointer) -> bool { + const tinygltf::URICallbacks *uri_cb, std::string *out_uri, + void *user_pointer) -> bool { + (void)basepath; + (void)image; + (void)uri_cb; REQUIRE(*filename == "foo"); REQUIRE(embedImages == true); REQUIRE(user_pointer == (void *)0xba5e1e55); @@ -616,7 +698,15 @@ auto writer = [](const std::string *basepath, const std::string *filename, const tinygltf::Image *image, bool embedImages, - std::string *out_uri, void *user_pointer) -> bool { + const tinygltf::URICallbacks *uri_cb, std::string *out_uri, + void *user_pointer) -> bool { + (void)basepath; + (void)filename; + (void)image; + (void)embedImages; + (void)uri_cb; + (void)out_uri; + (void)user_pointer; return false; };
diff --git a/tiny_gltf.h b/tiny_gltf.h index 5be8fa3..4e6825e 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h
@@ -1208,6 +1208,39 @@ }; /// +/// URIEncodeFunction type. Signature for custom URI encoding of external +/// resources such as .bin and image files. Used by tinygltf to re-encode the +/// final location of saved files. object_type may be used to encode buffer and +/// image URIs differently, for example. See +/// https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#uris +/// +typedef bool (*URIEncodeFunction)(const std::string &in_uri, + const std::string &object_type, + std::string *out_uri, void *user_data); + +/// +/// URIDecodeFunction type. Signature for custom URI decoding of external +/// resources such as .bin and image files. Used by tinygltf when computing +/// filenames to write resources. +/// +typedef bool (*URIDecodeFunction)(const std::string &in_uri, + std::string *out_uri, void *user_data); + +// Declaration of default uri decode function +bool URIDecode(const std::string &in_uri, std::string *out_uri, + void *user_data); + +/// +/// A structure containing URI callbacks and a pointer to their user data. +/// +struct URICallbacks { + URIEncodeFunction encode; // Optional encode method + URIDecodeFunction decode; // Required decode method + + void *user_data; // An argument that is passed to all uri callbacks +}; + +/// /// LoadImageDataFunction type. Signature for custom image loading callbacks. /// typedef bool (*LoadImageDataFunction)(Image *, const int, std::string *, @@ -1223,7 +1256,9 @@ typedef bool (*WriteImageDataFunction)(const std::string *basepath, const std::string *filename, const Image *image, bool embedImages, - std::string *out_uri, void *user_pointer); + const URICallbacks *uri_cb, + std::string *out_uri, + void *user_pointer); #ifndef TINYGLTF_NO_STB_IMAGE // Declaration of default image loader callback @@ -1235,8 +1270,8 @@ #ifndef TINYGLTF_NO_STB_IMAGE_WRITE // Declaration of default image writer callback bool WriteImageData(const std::string *basepath, const std::string *filename, - const Image *image, bool embedImages, std::string *out_uri, - void *); + const Image *image, bool embedImages, + const URICallbacks *uri_cb, std::string *out_uri, void *); #endif /// @@ -1389,6 +1424,11 @@ void SetImageWriter(WriteImageDataFunction WriteImageData, void *user_data); /// + /// Set callbacks to use for URI encoding and decoding and their user data + /// + void SetURICallbacks(URICallbacks callbacks); + + /// /// Set callbacks to use for filesystem (fs) access and their user data /// void SetFsCallbacks(FsCallbacks callbacks); @@ -1470,6 +1510,15 @@ #endif }; + URICallbacks uri_cb = { + // Use paths as-is by default. This will use JSON string escaping. + nullptr, + // Decode all URIs before using them as paths as the application may have + // percent encoded them. + &tinygltf::URIDecode, + // URI callback user data + nullptr}; + LoadImageDataFunction LoadImageData = #ifndef TINYGLTF_NO_STB_IMAGE &tinygltf::LoadImageData; @@ -2291,6 +2340,13 @@ } // namespace dlib // --- dlib end -------------------------------------------------------------- +bool URIDecode(const std::string &in_uri, std::string *out_uri, + void *user_data) { + (void)user_data; + *out_uri = dlib::urldecode(in_uri); + return true; +} + static bool LoadExternalFile(std::vector<unsigned char> *out, std::string *err, std::string *warn, const std::string &filename, const std::string &basedir, bool required, @@ -2501,7 +2557,8 @@ } bool WriteImageData(const std::string *basepath, const std::string *filename, - const Image *image, bool embedImages, std::string *out_uri, + const Image *image, bool embedImages, + const URICallbacks *uri_cb, std::string *out_uri, void *fsPtr) { const std::string ext = GetFilePathExtension(*filename); @@ -2563,13 +2620,26 @@ } else { // Throw error? } - *out_uri = *filename; + if (uri_cb->encode) { + if (!uri_cb->encode(*filename, "image", out_uri, uri_cb->user_data)) { + return false; + } + } else { + *out_uri = *filename; + } } return true; } #endif +void TinyGLTF::SetURICallbacks(URICallbacks callbacks) { + assert(callbacks.decode); + if (callbacks.decode) { + uri_cb = callbacks; + } +} + void TinyGLTF::SetFsCallbacks(FsCallbacks callbacks) { fs = callbacks; } #ifdef _WIN32 @@ -2836,13 +2906,19 @@ static bool UpdateImageObject(const Image &image, std::string &baseDir, int index, bool embedImages, + const URICallbacks *uri_cb, WriteImageDataFunction *WriteImageData, - std::string *out_uri, void *user_data) { + void *user_data, std::string *out_uri) { std::string filename; std::string ext; // If image has uri, use it as a filename if (image.uri.size()) { - filename = GetBaseFilename(image.uri); + std::string decoded_uri; + if (!uri_cb->decode(image.uri, &decoded_uri, uri_cb->user_data)) { + // A decode failure results in a failure to write the gltf. + return false; + } + filename = GetBaseFilename(decoded_uri); ext = GetFilePathExtension(filename); } else if (image.bufferView != -1) { // If there's no URI and the data exists in a buffer, @@ -2863,7 +2939,7 @@ bool imageWritten = false; if (*WriteImageData != nullptr && !filename.empty() && !image.image.empty()) { imageWritten = (*WriteImageData)(&baseDir, &filename, &image, embedImages, - out_uri, user_data); + uri_cb, out_uri, user_data); if (!imageWritten) { return false; } @@ -3793,6 +3869,7 @@ std::string *warn, const json &o, bool store_original_json_for_extras_and_extensions, const std::string &basedir, FsCallbacks *fs, + const URICallbacks *uri_cb, LoadImageDataFunction *LoadImageData = nullptr, void *load_image_user_data = nullptr) { // A glTF image must either reference a bufferView or an image uri @@ -3903,7 +3980,18 @@ #ifdef TINYGLTF_NO_EXTERNAL_IMAGE return true; #else - std::string decoded_uri = dlib::urldecode(uri); + std::string decoded_uri; + if (!uri_cb->decode(uri, &decoded_uri, uri_cb->user_data)) { + if (warn) { + (*warn) += "Failed to decode 'uri' for image[" + + std::to_string(image_idx) + "] name = [" + image->name + + "]\n"; + } + + // Image loading failure is not critical to overall gltf loading. + return true; + } + if (!LoadExternalFile(&img, err, warn, decoded_uri, basedir, /* required */ false, /* required bytes */ 0, /* checksize */ false, fs)) { @@ -4082,8 +4170,8 @@ static bool ParseBuffer(Buffer *buffer, std::string *err, const json &o, bool store_original_json_for_extras_and_extensions, - FsCallbacks *fs, const std::string &basedir, - bool is_binary = false, + FsCallbacks *fs, const URICallbacks *uri_cb, + const std::string &basedir, bool is_binary = false, const unsigned char *bin_data = nullptr, size_t bin_size = 0) { size_t byteLength; @@ -4129,7 +4217,10 @@ } } else { // External .bin file. - std::string decoded_uri = dlib::urldecode(buffer->uri); + std::string decoded_uri; + if (!uri_cb->decode(buffer->uri, &decoded_uri, uri_cb->user_data)) { + return false; + } if (!LoadExternalFile(&buffer->data, err, /* warn */ nullptr, decoded_uri, basedir, /* required */ true, byteLength, /* checkSize */ true, fs)) { @@ -4174,7 +4265,10 @@ } } else { // Assume external .bin file. - std::string decoded_uri = dlib::urldecode(buffer->uri); + std::string decoded_uri; + if (!uri_cb->decode(buffer->uri, &decoded_uri, uri_cb->user_data)) { + return false; + } if (!LoadExternalFile(&buffer->data, err, /* warn */ nullptr, decoded_uri, basedir, /* required */ true, byteLength, /* checkSize */ true, fs)) { @@ -5717,7 +5811,7 @@ Buffer buffer; if (!ParseBuffer(&buffer, err, o, store_original_json_for_extras_and_extensions_, &fs, - base_dir, is_binary_, bin_data_, bin_size_)) { + &uri_cb, base_dir, is_binary_, bin_data_, bin_size_)) { return false; } @@ -5988,7 +6082,8 @@ Image image; if (!ParseImage(&image, idx, err, warn, o, store_original_json_for_extras_and_extensions_, base_dir, - &fs, &this->LoadImageData, load_image_user_data)) { + &fs, &uri_cb, &this->LoadImageData, + load_image_user_data)) { return false; } @@ -6984,10 +7079,10 @@ static bool SerializeGltfBuffer(const Buffer &buffer, json &o, const std::string &binFilename, - const std::string &binBaseFilename) { + const std::string &binUri) { if (!SerializeGltfBufferData(buffer.data, binFilename)) return false; SerializeNumberProperty("byteLength", buffer.data.size(), o); - SerializeStringProperty("uri", binBaseFilename, o); + SerializeStringProperty("uri", binUri, o); if (buffer.name.size()) SerializeStringProperty("name", buffer.name, o); @@ -7031,7 +7126,6 @@ SerializeStringProperty("mimeType", image.mimeType, o); SerializeNumberProperty<int>("bufferView", image.bufferView, o); } else { - // TODO(syoyo): dlib::urilencode? SerializeStringProperty("uri", uri, o); } @@ -7825,8 +7919,8 @@ // we std::string uri; if (!UpdateImageObject(model->images[i], dummystring, int(i), true, - &this->WriteImageData, &uri, - this->write_image_user_data_)) { + &uri_cb, &this->WriteImageData, + this->write_image_user_data_, &uri)) { return false; } SerializeGltfImage(model->images[i], uri, image); @@ -7865,7 +7959,7 @@ SerializeGltfModel(model, output); // BUFFERS - std::vector<std::string> usedUris; + std::vector<std::string> usedFilenames; std::vector<unsigned char> binBuffer; if (model->buffers.size()) { json buffers; @@ -7878,27 +7972,40 @@ SerializeGltfBuffer(model->buffers[i], buffer); } else { std::string binSavePath; + std::string binFilename; std::string binUri; if (!model->buffers[i].uri.empty() && !IsDataURI(model->buffers[i].uri)) { binUri = model->buffers[i].uri; + if (!uri_cb.decode(binUri, &binFilename, uri_cb.user_data)) { + return false; + } } else { - binUri = defaultBinFilename + defaultBinFileExt; + binFilename = defaultBinFilename + defaultBinFileExt; bool inUse = true; int numUsed = 0; while (inUse) { inUse = false; - for (const std::string &usedName : usedUris) { - if (binUri.compare(usedName) != 0) continue; + for (const std::string &usedName : usedFilenames) { + if (binFilename.compare(usedName) != 0) continue; inUse = true; - binUri = defaultBinFilename + std::to_string(numUsed++) + - defaultBinFileExt; + binFilename = defaultBinFilename + std::to_string(numUsed++) + + defaultBinFileExt; break; } } + + if (uri_cb.encode) { + if (!uri_cb.encode(binFilename, "buffer", &binUri, + uri_cb.user_data)) { + return false; + } + } else { + binUri = binFilename; + } } - usedUris.push_back(binUri); - binSavePath = JoinPath(baseDir, binUri); + usedFilenames.push_back(binFilename); + binSavePath = JoinPath(baseDir, binFilename); if (!SerializeGltfBuffer(model->buffers[i], buffer, binSavePath, binUri)) { return false; @@ -7918,8 +8025,8 @@ std::string uri; if (!UpdateImageObject(model->images[i], baseDir, int(i), embedImages, - &this->WriteImageData, &uri, - this->write_image_user_data_)) { + &uri_cb, &this->WriteImageData, + this->write_image_user_data_, &uri)) { return false; } SerializeGltfImage(model->images[i], uri, image);