There are no messages displayed if install_node_export_import_from_file() fails, e.g. because the file either did not exist or could not be loaded.

CommentFileSizeAuthor
#1 install_profile_api-n402642-export_errors.patch524 bytesDamienMcKenna
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Active » Needs review
FileSize
install_profile_api-n402642-export_errors.patch524 bytes

The attached file adds a dsm() to indicate the file could not be found or opened.

James Andres’s picture

James Andres CreditAttribution: James Andres commented
Status: Needs review » Fixed

Seems sensible to me. Committed.

dww’s picture

dww
we/he/they
CreditAttribution: dww commented
Status: Fixed » Needs work

where does the value of $file come from? %file or @file would be safer than !file.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

dww: $file is one of the arguments.

dww’s picture

dww
we/he/they
CreditAttribution: dww commented

That I understand. My point is where is $file coming from when this function is called? I know we're talking about an install profile, and therefore, user-supplied data isn't really a concern, but in general, it's better to be in the habit of writing secure code by default. Maybe this function is going to be reused in some context we don't anticipate. Better safe than sorry.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

dww: the command would be executed like this:

// load the about_us page
install_node_export_import_from_file('about_us.inc', array('status'=>1), $user, FALSE);

So the $file would be manually loaded by the admin.

dww’s picture

dww
we/he/they
CreditAttribution: dww commented

Sorry, I keep phrasing my comments in the form of a question. I don't have a question at all. ;) I'm making a statement. Let me be more clear:

API functions shouldn't assume they know exactly how they're going to be called at all times in the future, so they should be written to be as safe as possible for any possible use-case. API functions that generate HTML sent to a browser should always sanitize potentially user-specified data using core's existing text filtering functions.

James Andres’s picture

James Andres CreditAttribution: James Andres commented

dww, I follow and agree.

I propose we consider a standard format for exported content that works with the install_profile_api project. Maybe the format is just a wrapper around the original export (eg: from views, content_copy, etc..). A standard format would have the following advantages, and disadvantages:

Advantages

  1. Exports can be parsed for sanity before import is attempted
  2. Files that contain exports can be detected, ie: a function like install_get_exports($type, $path) could be created
  3. Exports could be, more easily and safely, imported in batches
  4. Multiple exports could be contained in a single file, if the format we create supports this

Disadvantages

  1. Exports will be more difficult to create because, currently, each export follows a different style. All those styles will have to be merged, or wrapped.

Thoughts?

anarcat’s picture

anarcat CreditAttribution: anarcat commented
Status: Needs work » Postponed (maintainer needs more info)

I noticed this commit while doing the latest release notes... so I was under the impression this bug was fixed... could you try again with the latest release?

anarcat’s picture

anarcat CreditAttribution: anarcat commented
Status: Postponed (maintainer needs more info) » Needs work

Nevermind me, i got confused by the date. I'll remove this bug from the relnotes, there clearly seems to be issues remaining to discuss.