Demo page truncates first character of relative URL

Closing old issues,
Pretty sure this can't be reproduced any more. I've never seen it.

I'm reopening this issue because I have the same problem with version 7.x-1.0. The first character of a relative file path gets truncated on demo page:

Source file s/ites/default/files/garage/tasks/changingtheoil.html is not a normal file.

I think (due to file suffix '') that 's/ites/default/files/garage/tasks/changingtheoil.html' is not a html page I can process.

Frank

I've had a look at the code and my prime suspect is the ensure_trailing_slash() function in file-routines.inc. Looks like there's a quantifier missing.

/**
 * Adds a slash to a directory path if it wasn't already set
 * Unless input was an empty string.
 *
 * @parameter string $path
 * @returns string path.
 */
function ensure_trailing_slash($path) {
  return preg_replace( '|(.)/?$|', '$1/', $path );
}

Yeah, good spotting. Looks like that. Wonder how that lasted so long.

but then again - no, it's not that bad. just does it in an unexpected way. Hm.

Yes, on second thought the code looks right as the regex is anchored to the end of the string. I think I will have a closer look at the whole path-related code. There's quite some splitting, replacing and re-assembling going on ;-)

Just another observation from the extended error message:

Fetching content from local 's/ites/default/files/garage/tasks/changingtheoil.html/' now. Saving temp file locally as sites/default/files/imported/ites/default/files/garage/tasks/changingtheoil.html/index.html

The module doesn't seem to recognize the URL as pointing to a file in the first place but to a directory instead (see the added trailing slash to the URL and the addition of index.html to the temp file path). This is also indicated by the empty file suffix in the error message in #2.

My first idea was that the regex in the above mentioned ensure_trailing_slash() function does some unwanted backtracking but I cannot see from the code why it should do that. I'm investigating further ;-)

Looks like my first suspicion was correct, the culprit is indeed the ensure_trailing_slash() function. You can verify this by adding a marker character (#) after the inserted slash like so:

function ensure_trailing_slash($path) {
  return preg_replace( '|(.)/?$|', '$1/#', $path );
}

This will produce an error message like this one:

Source file s/#ites/default/files/garage/tasks/changingtheoil.html is not a normal file.

However, testing the regex doesn't show any error but when removing the end-of-line anchor ($) I can reproduce the shown behavior. So preg_replace() seems to ignore the anchor.

Will investigate further in that direction and hope to provide a patch soon.

Frank

EDIT:

• http://php.net/manual/en/function.preg-replace.php
• http://php.net/manual/en/regexp.reference.anchors.php
• http://php.net/manual/en/reference.pcre.pattern.differences.php

Here's another observation. The first character seems to be stripped from the path as can be seen from the following error message.

Importing 'ites/default/files/garage/tasks/changingtheoil.html'	

I think (due to file suffix '') that 's/#ites/default/files/garage/tasks/changingtheoil.html' is not a html page I can process.

It looks as if the path is split, then the ensure_trailing_slash() function adds a slash to the first part ($rel_path) correctly, and then the whole path is re-assembled again. Therefore I'm now suspecting something wrong with the following code from import_html_ui.inc:

...
// Divide the path into two halves (avoiding glue_url())
// to provide a dummy context and a rel_path
$split_at = strrpos($source_path, $url_parts['path']) + 1;
$form_state['values']['source_siteroot'] = $source_siteroot = substr($source_path, 0, $split_at);
$rel_path = substr($source_path, $split_at);
...

This also still leaves the problem why the file extension isn't recognized in the first place.

Frank

Getting closer, I think. The module uses parse_url() for parsing the input path (import_html_ui.inc, line 2125):

...
$url_parts = parse_url($source_path);
...

But according to http://php.net/manual/en/function.parse-url.php "This function doesn't work with relative URLs."

There might be a workaround by converting the relative path to an absolute one (see http://stackoverflow.com/questions/4444475/transfrom-relative-path-into-...) or to use realpath() instead.

Thanks for the follow-ups.
If it's *just* the demo page functionality and just the internal links when using the demo, I can easily imagine how that would have slipped through.
That page has only been used for checking the field-mappings and general shape of the page. Once upon a time (D5) it was possible to use the 'save' button and import a page one-by-one, but I stopped using that feature due to ... problems getting the context right when just hitting one page at a time :-}
Image support etc for those was particularly problematic, and had to use different rewrite rules that a real import would in order to pretend to work.

So yeah, some examples would have been sorta-OK when the remote site used absolute URLS. But a remote site with relative URLs ... would have been tricky to sort out. Not impossible or anything, just probably not tested too hard there.

Thanks for the feedback.

This only seems to affect the demo page functionality and only when using a relative path. AFAICS there are at least two workarounds for this issue:

1. Use an absolute path with the demo page feature.
2. Import only a single page with the normal import feature.

Therefore I tend to regard this issue as "Won't fix". Perhaps a hint in the UI to use only absolute paths with the demo feature would be helpful. What do you think?

Kind regards,
Frank

Well, now that you've narrowed it all the way down to there, I don't think it's a "Won't fix"!
If it's reliably replicable, and just to do with the path re-joining, then a fix is therefore possible.
I did try to note that the demo was just that - a demo (of the scraping, not the re-linking) - and may not be suitable as a base for ongoing imports and getting links exactly right. BUT I would have liked if that were possible.
It's just that trying to calculate whether a local relative link should assume that its target is also being imported (true in bulk, false in demo) was tricky to solve.

I'm sure that a solution for the string truncation issue is now in sight.