HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Please upgrade here. These earlier versions are no longer being updated and have security issues.

"Whoops there was an error." when trying to delete attachments.

This discussion is related to the fileupload addon.
DoyceTDoyceT Model Questioner ✭✭✭

Hi all,

Running 2.1.3 stable, version Version 1.8.2.1 of FileUpload (the one with Peregrine fixes), I'm getting the above error message when trying to delete files. I can open and/or download the attachments just fine.

I've gone digging through the forums for similar problems from other users, but haven't found anything yet.

Comments

  • maybe its the changes you made to change the name? does it work properly without your extra modifications that you made.

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

  • DoyceTDoyceT Model Questioner ✭✭✭

    Nope. That doesn't seem to have any effect. It knows what the file is called, the delete just... fails. Kind of odd.

  • vrijvlindervrijvlinder Papillon-Sauvage MVP
    edited September 2014

    do you have a file called blank.php in the fileupload views folder ? I ask because this is what the class.fileupload.plugin.php has for deletion..

    if ($Media) {
             $Delete['Media'] = $Media;
             $UserID = GetValue('UserID', Gdn::Session());
             if (GetValue('InsertUserID', $Media, NULL) == $UserID || Gdn::Session()->CheckPermission($Permission, TRUE, 'Category', $PermissionCategoryID)) {
                $this->MediaModel()->Delete($Media, TRUE);
                $Delete['Status'] = 'success';
             } else {
                throw PermissionException();
             }
          } else {
             throw NotFoundException('Media');
          }
    
          $Sender->SetJSON('Delete', $Delete);
          $Sender->Render($this->GetView('blank.php'));
       }
    
  • DoyceTDoyceT Model Questioner ✭✭✭

    I do have a blank.php file in the FileUpload/Views folder. It is, predictably, blank. :)

  • hgtonighthgtonight ∞ · New Moderator
    edited September 2014

    Try changing the $Sender->Render line to $Sender->RenderData();.

    EDIT - Also, does enabling debugging show an error message?

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • DoyceTDoyceT Model Questioner ✭✭✭

    I'll have to enable it over the weekend, when it's quieter. Turing on debug during classes generates dozens of panicked emails.

  • hgtonighthgtonight ∞ · New Moderator

    @DoyceT said:
    I'll have to enable it over the weekend, when it's quieter. Turing on debug during classes generates dozens of panicked emails.

    For some reason, this is hilarious to me.

    It isn't really funny, but I am laughing at the thought.

    Good on you for thinking ahead.

    If you want to "take the site down" in a nice way, check out Admin Mode: http://vanillaforums.org/addon/adminmode-plugin

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

    DoyceT
  • DoyceTDoyceT Model Questioner ✭✭✭

    Ooh. Ohh. That's handy. I'll be having that. Thanks!

  • DoyceTDoyceT Model Questioner ✭✭✭
    edited September 2014

    @hgtonight said:
    Try changing the $Sender->Render line to $Sender->RenderData();.

    EDIT - Also, does enabling debugging show an error message?

    Sorry, I'm being particularly dense about this one. Are you saying to switch this:

    $Sender->Render($this->GetView('blank.php'));
    

    to this?

    $Sender->RenderData();
    
  • DoyceTDoyceT Model Questioner ✭✭✭

    Actually, there are quite a few instances of $Sender->Render throughout the file - do you mean I should only change the one @vrijvlinder‌ mentioned?

  • hgtonighthgtonight ∞ · New Moderator

    Just the one in question.

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • DoyceTDoyceT Model Questioner ✭✭✭

    Thanks!

    Doesn't seem to have any effect, unfortunately.

    Is it possible that the "build filename" change I made over here - http://vanillaforums.org/discussion/comment/214712/#Comment_214712 - is breaking something? I doesn't seem like it should...

  • peregrineperegrine MVP
    edited September 2014

    it seems there may be a problem in 2.1.3 of vanilla with fileupload 1.8.2.1 as far as deleting an upload, after comment or discussion is saved.

    FATAL ERROR IN: Gdn_Controller.DeliveryType();

    Attempted to set invalid DeliveryType value (JSON)."

    LOCATION: /var/www/vanilla/library/core/class.controller.php
    

    714: if (defined('DELIVERY_TYPE_'.$Default)) {
    715: $this->_DeliveryType = $Default;
    716: }
    717: else {

    718: throw new Exception(sprintf(T('Attempted to set invalid DeliveryType value (%s).'), $Default));

    I'll see if I can track it down if no one else finds how to solve it.

    I'm fairly certain it used to delete properly in vanilla 2.1

    However, it appears there have been a few changes in 2.1.3 - that may be related to problem now.

    not sure at the moment how to resolve it via the plugin though. If I do find a solution, I'll post it, if I don't post a solution, its because I don't have one.

    https://github.com/vanilla/vanilla/commit/a69fa4048bd299d3248f0ff2897cde5e0ea93065

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

    DoyceT
  • peregrineperegrine MVP
    edited September 2014

    I need some help here. from others. @Doycet -- sorry I don't understand the concept with the whitelist, etc.

    if I set Default to "ALL" - then the file will get deleted. but I don't know the intent or where to set the call to DeliveryType to "ALL" before it gets to the function.

    its as if DeliveryMethod and DeliveryType are mixed up.

     public function DeliveryType($Default = '') {
        //  Test setting $Default to "ALL"
         if($Default == "JSON" )  $Default = "ALL";
          if ($Default) {
             // Make sure we only set a defined delivery type.
             // Use constants' name pattern instead of a strict whitelist for forwards-compatibility.
             if (defined('DELIVERY_TYPE_'.$Default)) {
                $this->_DeliveryType = $Default;
             }
             else {
                throw new Exception(sprintf(T('Attempted to set invalid DeliveryType value (%s).'), $Default));
             }
          }
    
          return $this->_DeliveryType;
       }
    

    perhaps x00, Linc, or Hg will know.

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

    DoyceT
  • DoyceTDoyceT Model Questioner ✭✭✭

    If I have no other use, at least I'm a semi-reliable landmine locator - by which I mean I'm glad I might have found a bug?

    peregrine
  • BleistivtBleistivt MVP
    edited September 2014

    @‌peregrine It's the ajax request, that tries to set an invalid DeliveryType

    edit js/fileupload.js line 120:

           deliveryType: 'VIEW',
    

    Not sure if View is exactly right, but at least it's not invalid

    I believe versions prior to 2.1.2 (2.1.3) did not throw a fatal error on invalid DeliveryTypes.

    peregrine
  • peregrineperegrine MVP
    edited September 2014

    @Bleistivt said:

    I believe versions prior to 2.1.2 (2.1.3) did not throw a fatal error on invalid DeliveryTypes.

    thx. yes it was the commit 24 days ago that changed things.

    they changed it here as well

    https://github.com/vanilla/vanilla/blob/a69fa4048bd299d3248f0ff2897cde5e0ea93065/applications/dashboard/js/addons.js#L39

    https://github.com/vanilla/vanilla/commit/a69fa4048bd299d3248f0ff2897cde5e0ea93065

    and bingo changing it to VIEW from JSON works.

    @Bleistivt said:
    @‌peregrine It's the ajax request, that tries to set an invalid DeliveryType

    edit js/fileupload.js line 120:

           deliveryType: 'VIEW',
    

    nice.

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

    Adrian
  • peregrineperegrine MVP
    edited September 2014

    @DoyceT said:
    If I have no other use, at least I'm a semi-reliable landmine locator - by which I mean I'm glad I might have found a bug?

    yes you can sniff them out. nice find relating to bug in Fileuploads 1.8.2.1 plugin in 2.1.3 (that was not an issue in 2.1

    I'll put a note in the other discussion.
    http://vanillaforums.org/discussion/comment/216335/#Comment_216335

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

  • AdrianAdrian Wandering Spirit Montreal Vanilla Staff

    I've submitted the changes to Github for review (referencing this discussion). That's the best way to make sure it gets fixed :)

    DoyceT
  • JasonBarnabeJasonBarnabe Cynical Salamander ✭✭

    Just to save others a bit of time, @Adrian's PR is https://github.com/vanilla/addons/pull/139.

    DoyceThgtonightAdrian
Sign In or Register to comment.