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.

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

Bug : File Loading Through PHP Redirection not working #507

Open
pthomas-44 opened this issue Nov 7, 2024 · 1 comment
Open

Bug : File Loading Through PHP Redirection not working #507

pthomas-44 opened this issue Nov 7, 2024 · 1 comment

Comments

@pthomas-44
Copy link

pthomas-44 commented Nov 7, 2024

Describe the bug

I encountered the bug while installing a local version of the website for local usage.
When attempting to load a 3D file in Online 3D Viewer using a URL with PHP redirection, the viewer encounters an error. Specifically, when a file link includes a PHP redirect (e.g., http://mynas/mywiki/index.php?title=Special:Redirect/file/model.step), the GetFileContent() function fails to handle the redirection properly. While it updates the content field of the file object, it does not modify the data, name, and extension fields, resulting in an incomplete file load.

To Reproduce

Set up Online 3D Viewer.
Attempt to load a STEP file via a link that includes a PHP redirect (such as https://viapixnax/mediawiki/index.php?title=Special:Redirect/file/2418-0003.step).

Expected behavior

The GetFileContent() function should handle PHP redirection by correctly updating the data, name, and extension fields of the file object, allowing the file to load fully in the viewer without errors.

Screenshots

300px-O3dv_error_window_php
300px-O3dv_error_console_php

@pthomas-44
Copy link
Author

pthomas-44 commented Nov 7, 2024

As a quickfix I applied the following modifications into the source code (which worked):

To solve the problem, I modified the RequestUrl() function by passing the response object to the resolution function instead of simply sending the file content. Additionally, I updated the resolution function's code to ensure it correctly updates the necessary fields in the file object.

PS : As I have never contributed to anything on GitHub, I am kind of lost, to be honest. I would greatly appreciate it if someone could explain to me how to create a pull request and submit my fix after discussing the problem.

In engine/io/fileutils.js:

export function RequestUrl (url, onProgress)
{
	return new Promise ((resolve, reject) => {
		let request = new XMLHttpRequest ();
		request.open ('GET', url, true);

		request.onprogress = (event) => {
			onProgress (event.loaded, event.total);
		};

		request.onload = () => {
			if (request.status === 200) {
				resolve (request); // Old code : resolve(request.response);
			} else {
				reject ();
			}
		};

		request.onerror = () => {
			reject ();
		};

		request.responseType = 'arraybuffer';
		request.send (null);
	});
}

In engine/import/importerfiles.js:

GetFileContent (file, callbacks)
{
    if (file.content !== null) {
        callbacks.onReady ();
        return;
    }
    let loaderPromise = null;
    if (file.source === FileSource.Url) {
        loaderPromise = RequestUrl (file.data, callbacks.onProgress);
    } else if (file.source === FileSource.File) {
        loaderPromise = ReadFile (file.data, callbacks.onProgress);
    } else {
        callbacks.onReady ();
        return;
    }
    loaderPromise.then ((content) => {                          // With the modifications made to the RequestUrl() function,
        if (file.source === FileSource.Url) {                   // `content` now contains a `request` object rather than
              file.SetContent (content.response);               // the actual file content when loading via URL.
              file.data = content.responseURL;                  // However, in the case of loading from a local machine, `content`
              file.name = content.responseURL.split('/').pop(); // still corresponds to the file content,
              file.extension = file.name.split('.').pop();      // so it's important to add the condition to maintain
        } else {                                                // the proper functionality for loading from a local machine.
              file.SetContent (content);
        }
    }).catch (() => {
    }).finally (() => {
        callbacks.onReady ();
    });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant