View Issue Details

This bug affects 3 person(s).
 28
IDProjectCategoryView StatusLast Update
18143Bug reports_ Unknownpublic2022-08-19 14:39
Reporterritapas Assigned Toollehar  
PrioritynoneSeveritypartial_block 
Status assignedResolutionopen 
Product Version3.28.x 
Summary18143: cannot import resources for imported (lsa) survey with uploaded resources in responses
Description

if you export (lsa) a survey with uploaded resources and also import the related resources, such import fails

Steps To Reproduce

Steps to reproduce

create survey with resources in question and a file upload question.
execute at least once with a file upload
export survey as lsa and resources as zip
create new survey from lsa and import zip resources

Expected result

resources appear in question and in answers

Actual result

resources import throws a warning.
resources appear in question but not in answers

also happens in 3.28.12

TagsNo tags attached.
Attached Files
ImportResources.png (23,233 bytes)   
ImportResources.png (23,233 bytes)   
Bug heat28
Complete LimeSurvey version number (& build)3.28.8 (build 220426)
I will donate to the project if issue is resolvedNo
BrowserMozilla Firefox
Database type & versionphp-mysql-5.4.16
Server OS (if known)Red Hat Enterprise Linux Server release 7.5 (Maipo)
Webserver software & version (if known)httpd-2.4.6-80
PHP Versionrh-php71

Users monitoring this issue

jamdome, ritapas, seccanj

Activities

DenisChenu

DenisChenu

2022-05-24 08:38

developer   ~69994

The old big issue to set uploaded file in resources …
really a big issue here …

gabrieljenik

gabrieljenik

2022-06-30 20:39

manager   ~70640

PR: https://github.com/LimeSurvey/LimeSurvey/pull/2494

DenisChenu

DenisChenu

2022-06-30 21:37

developer   ~70652

Last edited: 2022-06-30 21:39

But : user file list are pot part of survey resources !

I really think we mist not import : it's part of response not resoures

The fix must be

  • export resource didn't export fu_ file

New feature

  • export user file export fu_ file
  • import user file
gabrieljenik

gabrieljenik

2022-06-30 21:44

manager   ~70653

If fileuploads are exported on resources now and app is intending to upload them, the bugfixing would be to allow them to be imported.

it's part of response not resoures

From my point of view, that is a reorganization, another ticket

DenisChenu

DenisChenu

2022-06-30 21:46

developer   ~70655

Personally i clearly not merge like this …

And it don't fix the issue …
Set ressource upload allowed to png,jpg,gif
set file upload allowed to doc, docx, odt

You don't import the uploaded files ⋅

DenisChenu

DenisChenu

2022-06-30 21:47

developer   ~70657

From my point of view, that is a reorganization, another ticket

fu_ file is part of response, it's not a point of view here, clearly not …

gabrieljenik

gabrieljenik

2022-06-30 21:49

manager   ~70659

@c_schmitz... what do you think?

gabrieljenik

gabrieljenik

2022-06-30 21:58

manager   ~70661

fu_ file is part of response,

I agree with that.
The discusion is about when to make them exportable/importable.

I believe is a new feature. You said it also.
So the current ticket needs to find a way to allow the current feature implemented to work as expected.
That is allow people to upload fu_ files with the resources.

The fix must be: export resource didn't export fu_ file

So the fix is to "not allow"?

DenisChenu

DenisChenu

2022-07-01 08:32

developer   ~70664

So the fix is to "not allow"?

No : the fix is don't export user files as survey resources :)

DenisChenu

DenisChenu

2022-07-01 17:15

developer   ~70681

Gabriel an Me are unsurfe on the fix to do.

Me : don't export : it's not survey ressource (and create a new fetaire import/export user file)
Gabriel : leave it for export, then try to import it

PS : in general : whe n i have user files : size of zip get more than 50 Mo … i prefer to don't have it in ressources …

ollehar

ollehar

2022-07-29 18:38

administrator   ~71246

Yeah, I'm a bit unsure. Gabriel's fix has no immediate security issue or danger to it, tho, right? But as Denis remarks, it's not 100% tight either, due to different settings in permissions (resource file upload vs question file upload). I guess we can both merge this and plan a more solid solution together?

ollehar

ollehar

2022-07-29 18:39

administrator   ~71247

@DenisChenu, do you want to remove browsing response files in the resource file manager? Or what's your idea there?

DenisChenu

DenisChenu

2022-07-29 18:41

developer   ~71248

OK, ready for testing

DenisChenu

DenisChenu

2022-07-29 19:02

developer   ~71249

1st : allow to expprt wothout user file (second button)

But what i want really ? Allow to save on another directory : save out of web access. It's a bug reported since years now …

User with apach have .htaccess, user with nginx or IIS are not protected

    #Disallow direct read user upload files
    location ~ ^/upload/surveys/.*/fu_[a-z0-9]*$ {
        return 444;
    }
gabrieljenik

gabrieljenik

2022-07-29 19:10

manager   ~71251

@ritapas maybe you can help by testing the PR?
https://github.com/LimeSurvey/LimeSurvey/pull/2494

ollehar

ollehar

2022-07-29 19:39

administrator   ~71252

@DenisChenu, you can put uploaddir outside your web dir, no? In config.php.

DenisChenu

DenisChenu

2022-07-29 20:38

developer   ~71253

@DenisChenu, you can put uploaddir outside your web dir, no? In config.php.

No because resource file is used for img source or PDF link in survey.

upload/survey MUST be in web dir
upload/theme can be out of web dir IF debug is not set (debug + registerCss : file need to be in web dir)

ritapas

ritapas

2022-08-01 11:08

reporter   ~71261

@gabrieljenik is the fix inside last release?

ollehar

ollehar

2022-08-01 11:09

administrator   ~71262

No because resource file is used for img source or PDF link in survey.

OK, got it

gabrieljenik

gabrieljenik

2022-08-01 20:07

manager   ~71282

@gabrieljenik is the fix inside last release?

@ritapas No, in this PR
https://github.com/LimeSurvey/LimeSurvey/pull/2494

I think you have dealt with PRs before, right?

Thanks

ritapas

ritapas

2022-08-02 08:39

reporter   ~71286

@gabrieljenik sorry no, but I will seek for help

ollehar

ollehar

2022-08-03 13:08

administrator   ~71306

@ritapas You can also give us the lss and zip file needed to do the test.

ritapas

ritapas

2022-08-03 14:22

reporter   ~71307

@ollehar sure, I am glad to help.
Here you are the lsa and zip file.
The problem indeed is when importing resources that participants send us as "file uploads".

test export resources.zip (496,601 bytes)
ritapas

ritapas

2022-08-08 17:30

reporter   ~71385

Hello,
I managed to incorporate the new code in our environment.
I seems to be working fine, except for complaining about the index.html file.
Is there a reason for it always being added to resources and the being impossible to upload back?

gabrieljenik

gabrieljenik

2022-08-09 22:01

manager   ~71393

@ollehar Maybe we can allow html?

ollehar

ollehar

2022-08-10 10:19

administrator   ~71394

Not sure about XSS there, since HTML also includes script tags and external links.

gabrieljenik

gabrieljenik

2022-08-10 13:55

manager   ~71403

Well. JS is already accepted.

If not, how shall it be handled the latest comment from ritapas?

ritapas

ritapas

2022-08-10 14:02

reporter   ~71404

as far as I have seen since now, the index.html only contains the following:

%%
<html><head></head><body></body></html>
%%

ritapas

ritapas

2022-08-10 14:02

reporter   ~71405

sorry, I meant
<html><head></head><body></body></html>

but I thought that needed some escaping to avoid tag parsing

gabrieljenik

gabrieljenik

2022-08-11 20:00

manager   ~71415

@ollehar will leave it up to you.

Should we allow it or wrap up the fix without allowing it?

ollehar

ollehar

2022-08-12 10:28

administrator   ~71427

Pinging Carsten for comment.

c_schmitz

c_schmitz

2022-08-12 11:34

administrator   ~71431

Last edited: 2022-08-12 11:35

I think the whole PR is not the proper way to fix it.
IMHO response files should be part of survey archive export/import as an option, not of survey resources.

ritapas

ritapas

2022-08-12 11:43

reporter   ~71432

i would agree

gabrieljenik

gabrieljenik

2022-08-12 15:35

manager   ~71433

response files should be part of survey archive export/import as an option, not of survey resources.

I agree that FU should be separated from responses, but that's a concept change.

I think the whole PR is not the proper way to fix it.

Obvisouly don't agree :) haha
I understand is not the long term fix.
But more like a fix according to the current situation and current concepts.

The bug is: with the current concepts, why it doesn't work?
The PR is a fix for it, allowing the current implemented feature to work as expected.

response files should be part of survey archive export/import as an option, not of survey resources.

We could do another ticket for allowing that, but would be a bigger change.
Not a fix in the code, but a change in the expectations.
That's why it would be a another ticket,

Question is: If this allows the current implemented feature to work, why would we need to block it?
Can;t we just release it and then prioritize the bigger change?

DenisChenu

DenisChenu

2022-08-17 11:40

developer   ~71460

Maybe best is to do not add fu_XXX file in resource zip export for the 1st PR ?

I'm OK to fix it like this (next week)

gabrieljenik

gabrieljenik

2022-08-17 15:19

manager   ~71464

Maybe best is to do not add fu_XXX file in resource zip export for the 1st PR ?

Then the FU will not be exportable at all.

gabrieljenik

gabrieljenik

2022-08-19 14:39

manager   ~71495

Comments from chat iwth Cartsen:
What is missing, besides the place where the import is happening, ...

  • Validation file belongs to a response
  • If orphan files are uploaded, never be able to delete theme as no browser

Considerations:

  • What happens if you activate and deactivate.
  • Solution needs to be backward compatible.

Issue History

Date Modified Username Field Change
2022-05-23 19:09 ritapas New Issue
2022-05-23 19:09 ritapas File Added: ImportResources.png
2022-05-24 08:38 DenisChenu Note Added: 69994
2022-05-24 08:38 DenisChenu Bug heat 0 => 2
2022-06-09 13:54 seccanj Issue Monitored: seccanj
2022-06-09 13:54 seccanj Bug heat 2 => 10
2022-06-09 18:16 gabrieljenik Assigned To => gabrieljenik
2022-06-09 18:16 gabrieljenik Status new => assigned
2022-06-17 10:06 guest Bug heat 10 => 16
2022-06-17 10:06 jamdome Issue Monitored: jamdome
2022-06-17 10:06 jamdome Bug heat 16 => 18
2022-06-22 14:59 ritapas Issue Monitored: ritapas
2022-06-22 14:59 ritapas Bug heat 18 => 20
2022-06-30 20:39 gabrieljenik Status assigned => ready for code review
2022-06-30 20:39 gabrieljenik Note Added: 70640
2022-06-30 20:39 gabrieljenik Bug heat 20 => 22
2022-06-30 20:39 gabrieljenik Assigned To gabrieljenik => DenisChenu
2022-06-30 21:37 DenisChenu Note Added: 70652
2022-06-30 21:38 DenisChenu Note Edited: 70652
2022-06-30 21:39 DenisChenu Note Edited: 70652
2022-06-30 21:44 gabrieljenik Note Added: 70653
2022-06-30 21:46 DenisChenu Note Added: 70655
2022-06-30 21:47 DenisChenu Note Added: 70657
2022-06-30 21:49 gabrieljenik Note Added: 70659
2022-06-30 21:58 gabrieljenik Note Added: 70661
2022-07-01 08:32 DenisChenu Note Added: 70664
2022-07-01 17:15 DenisChenu Assigned To DenisChenu => c_schmitz
2022-07-01 17:15 DenisChenu Status ready for code review => confirmed
2022-07-01 17:15 DenisChenu Note Added: 70681
2022-07-01 17:15 DenisChenu Status confirmed => in code review
2022-07-29 18:38 ollehar Note Added: 71246
2022-07-29 18:38 ollehar Bug heat 22 => 24
2022-07-29 18:39 ollehar Note Added: 71247
2022-07-29 18:41 DenisChenu Assigned To c_schmitz =>
2022-07-29 18:41 DenisChenu Status in code review => ready for testing
2022-07-29 18:41 DenisChenu Note Added: 71248
2022-07-29 19:02 DenisChenu Note Added: 71249
2022-07-29 19:10 gabrieljenik Note Added: 71251
2022-07-29 19:39 ollehar Note Added: 71252
2022-07-29 20:38 DenisChenu Note Added: 71253
2022-08-01 11:08 ritapas Note Added: 71261
2022-08-01 11:08 ritapas Bug heat 24 => 26
2022-08-01 11:09 ollehar Note Added: 71262
2022-08-01 20:07 gabrieljenik Note Added: 71282
2022-08-02 08:39 ritapas Note Added: 71286
2022-08-03 13:08 ollehar Note Added: 71306
2022-08-03 13:09 ollehar Assigned To => ollehar
2022-08-03 13:09 ollehar Status ready for testing => feedback
2022-08-03 14:22 ritapas Note Added: 71307
2022-08-03 14:22 ritapas File Added: test export resources.lsa
2022-08-03 14:22 ritapas File Added: test export resources.zip
2022-08-03 14:22 ritapas Status feedback => assigned
2022-08-08 17:30 ritapas Note Added: 71385
2022-08-08 17:30 ritapas File Added: Screenshot 2022-08-08 at 17-27-52 LimeSurvey DEV.png
2022-08-09 22:01 gabrieljenik Note Added: 71393
2022-08-10 10:19 ollehar Note Added: 71394
2022-08-10 13:55 gabrieljenik Note Added: 71403
2022-08-10 14:02 ritapas Note Added: 71404
2022-08-10 14:02 ritapas Note Added: 71405
2022-08-11 20:00 gabrieljenik Note Added: 71415
2022-08-12 10:28 ollehar Note Added: 71427
2022-08-12 11:34 c_schmitz Note Added: 71431
2022-08-12 11:34 c_schmitz Bug heat 26 => 28
2022-08-12 11:35 c_schmitz Note Edited: 71431
2022-08-12 11:43 ritapas Note Added: 71432
2022-08-12 15:35 gabrieljenik Note Added: 71433
2022-08-17 11:40 DenisChenu Note Added: 71460
2022-08-17 15:19 gabrieljenik Note Added: 71464
2022-08-19 14:39 gabrieljenik Note Added: 71495