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

dav: Fix handling of chunked WebDAV upload #13871

Merged
merged 1 commit into from
Mar 4, 2019
Merged

dav: Fix handling of chunked WebDAV upload #13871

merged 1 commit into from
Mar 4, 2019

Conversation

jplitza
Copy link
Contributor

@jplitza jplitza commented Jan 28, 2019

When $data is null (which can happen when $request->getBodyAsStream() returns null), the Exceptions says "copied bytes: 0, expected filesize: 0", which sounds more like success... So treat it as such!

This regression (it worked in Nextcloud 14) was probably introduced by commit 93de637

The setup resulting in this situation was a Canon iR-ADV C5030 uploading its scans via WebDAV to an SFTP external storage. Here is the error that resulted in an HTTP Error 500 being returned:

Nextcloud log (data/nextcloud.log)

Nextcloud log
{
  "reqId": "AQ84OEsk9APcLfSPb4tu",
  "level": 4,
  "time": "2019-01-28T12:36:20+00:00",
  "remoteAddr": "<redacted>",
  "user": "<redacted>",
  "app": "webdav",
  "method": "PUT",
  "url": "/remote.php/webdav/<redacted>",
  "message": {
    "Exception": "Sabre\\DAV\\Exception",
    "Message": "Error while copying file to target location (copied bytes: 0, expected filesize: 0 )",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/apps/dav/lib/Connector/Sabre/Directory.php",
        "line": 156,
        "function": "put",
        "class": "OCA\\DAV\\Connector\\Sabre\\File",
        "type": "->",
        "args": [
          null
        ]
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 1096,
        "function": "createFile",
        "class": "OCA\\DAV\\Connector\\Sabre\\Directory",
        "type": "->",
        "args": [
          "20190128125733.pdf",
          null
        ]
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/3rdparty/sabre/dav/lib/DAV/CorePlugin.php",
        "line": 525,
        "function": "createFile",
        "class": "Sabre\\DAV\\Server",
        "type": "->",
        "args": [
          "NAS/20190128125733.pdf",
          null,
          null
        ]
      },
      {
        "function": "httpPut",
        "class": "Sabre\\DAV\\CorePlugin",
        "type": "->",
        "args": [
          {
            "absoluteUrl": "https://<redacted>:443/remote.php/webdav/<redacted>",
            "__class__": "Sabre\\HTTP\\Request"
          },
          {
            "__class__": "Sabre\\HTTP\\Response"
          }
        ]
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/3rdparty/sabre/event/lib/EventEmitterTrait.php",
        "line": 105,
        "function": "call_user_func_array",
        "args": [
          [
            {
              "__class__": "Sabre\\DAV\\CorePlugin"
            },
            "httpPut"
          ],
          [
            {
              "absoluteUrl": "https://<redacted>:443/remote.php/webdav/<redacted>",
              "__class__": "Sabre\\HTTP\\Request"
            },
            {
              "__class__": "Sabre\\HTTP\\Response"
            }
          ]
        ]
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 479,
        "function": "emit",
        "class": "Sabre\\Event\\EventEmitter",
        "type": "->",
        "args": [
          "method:PUT",
          [
            {
              "absoluteUrl": "https://<redacted>:443/remote.php/webdav/<redacted>",
              "__class__": "Sabre\\HTTP\\Request"
            },
            {
              "__class__": "Sabre\\HTTP\\Response"
            }
          ]
        ]
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 254,
        "function": "invokeMethod",
        "class": "Sabre\\DAV\\Server",
        "type": "->",
        "args": [
          {
            "absoluteUrl": "https://<redacted>:443/remote.php/webdav/<redacted>",
            "__class__": "Sabre\\HTTP\\Request"
          },
          {
            "__class__": "Sabre\\HTTP\\Response"
          }
        ]
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/apps/dav/appinfo/v1/webdav.php",
        "line": 80,
        "function": "exec",
        "class": "Sabre\\DAV\\Server",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/vhosts/<redacted>/httpdocs/remote.php",
        "line": 163,
        "args": [
          "/var/www/vhosts/<redacted>/httpdocs/apps/dav/appinfo/v1/webdav.php"
        ],
        "function": "require_once"
      }
    ],
    "File": "/var/www/vhosts/<redacted>/httpdocs/apps/dav/lib/Connector/Sabre/File.php",
    "Line": 191,
    "CustomMessage": "--"
  },
  "userAgent": "UniversalSend_WebDAV/1.0",
  "version": "15.0.2.0"
}

When $data is null (which can happen when $request->getBodyAsStream() returns
null), the Exceptions says "copied bytes: 0, expected filesize: 0", which
sounds more like success...
@MorrisJobke
Copy link
Member

When $data is null (which can happen when $request->getBodyAsStream() returns null), the Exceptions says "copied bytes: 0, expected filesize: 0", which sounds more like success... So treat it as such!

Just to summarize:

this can be triggered by uploading an empty file?

@jplitza
Copy link
Contributor Author

jplitza commented Jan 29, 2019

@MorrisJobke No. The PDF uploaded by the scanner definitely isn't empty, and on the other hand uploading an empty TXT via gvfs didn't trigger the bug. I guess the scanner just starts with an empty body in the first request of a chunked transfer.

@MorrisJobke MorrisJobke mentioned this pull request Mar 4, 2019
45 tasks
@ariselseng ariselseng self-requested a review March 4, 2019 14:42
Copy link
Member

@ariselseng ariselseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes it for me. I triggered the bug by running touch to create an empty file and make the nextcloud desktop client sync it. I get http 500 without this fix.

@ariselseng
Copy link
Member

ariselseng commented Mar 4, 2019

this can be triggered by uploading an empty file?

@MorrisJobke You are right. I triggered this bug by syncing an empty file with the desktop client.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code makes sense 👍

@MorrisJobke
Copy link
Member

Status of 15220: failure

  • TESTS=acceptance, TESTS-ACCEPTANCE=app-comments
    • cancelled
  • TESTS=acceptance, TESTS-ACCEPTANCE=app-files-tags
    • tests/acceptance/features/app-files-tags.feature:70
Show full log
  Scenario: remove tags using the dropdown in the details view                                  # /drone/src/github.com/nextcloud/server/tests/acceptance/features/app-files-tags.feature:70
    Given I am logged in as the admin                                                           # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I visit the settings page                                                               # SettingsMenuContext::iVisitTheSettingsPage()
    And I open the "Workflow" section                                                           # AppNavigationContext::iOpenTheSection()
    And I see that the button to select tags is shown                                           # SettingsContext::iSeeThatTheButtonToSelectTagsIsShown()
    And I create the tag "tag1" in the settings                                                 # SettingsContext::iCreateTheTagInTheSettings()
    And I create the tag "tag2" in the settings                                                 # SettingsContext::iCreateTheTagInTheSettings()
    And I create the tag "tag3" in the settings                                                 # SettingsContext::iCreateTheTagInTheSettings()
    And I create the tag "tag4" in the settings                                                 # SettingsContext::iCreateTheTagInTheSettings()
    And I see that the dropdown for tags in the settings eventually contains the tag "tag1"     # SettingsContext::iSeeThatTheDropdownForTagsInTheSettingsEventuallyContainsTheTag()
    And I see that the dropdown for tags in the settings eventually contains the tag "tag2"     # SettingsContext::iSeeThatTheDropdownForTagsInTheSettingsEventuallyContainsTheTag()
    And I see that the dropdown for tags in the settings eventually contains the tag "tag3"     # SettingsContext::iSeeThatTheDropdownForTagsInTheSettingsEventuallyContainsTheTag()
    And I see that the dropdown for tags in the settings eventually contains the tag "tag4"     # SettingsContext::iSeeThatTheDropdownForTagsInTheSettingsEventuallyContainsTheTag()
    And I log out                                                                               # SettingsMenuContext::iLogOut()
    And I am logged in                                                                          # LoginPageContext::iAmLoggedIn()
    And I open the details view for "welcome.txt"                                               # FileListContext::iOpenTheDetailsViewFor()
    And I open the input field for tags in the details view                                     # FilesAppContext::iOpenTheInputFieldForTagsInTheDetailsView()
    And I check the tag "tag2" in the dropdown for tags in the details view                     # FilesAppContext::iCheckTheTagInTheDropdownForTagsInTheDetailsView()
    And I check the tag "tag4" in the dropdown for tags in the details view                     # FilesAppContext::iCheckTheTagInTheDropdownForTagsInTheDetailsView()
    And I check the tag "tag3" in the dropdown for tags in the details view                     # FilesAppContext::iCheckTheTagInTheDropdownForTagsInTheDetailsView()
    When I uncheck the tag "tag2" in the dropdown for tags in the details view                  # FilesAppContext::iUncheckTheTagInTheDropdownForTagsInTheDetailsView()
    And I uncheck the tag "tag4" in the dropdown for tags in the details view                   # FilesAppContext::iUncheckTheTagInTheDropdownForTagsInTheDetailsView()
    Then I see that the tag "tag2" in the dropdown for tags in the details view is not checked  # FilesAppContext::iSeeThatTheTagInTheDropdownForTagsInTheDetailsViewIsNotChecked()
    And I see that the tag "tag4" in the dropdown for tags in the details view is not checked   # FilesAppContext::iSeeThatTheTagInTheDropdownForTagsInTheDetailsViewIsNotChecked()
    And I see that the tag "tag3" in the dropdown for tags in the details view is checked       # FilesAppContext::iSeeThatTheTagInTheDropdownForTagsInTheDetailsViewIsChecked()
    And I see that the input field for tags in the details view does not contain the tag "tag2" # FilesAppContext::iSeeThatTheInputFieldForTagsInTheDetailsViewDoesNotContainTheTag()
      Failed asserting that false is true.
    And I see that the input field for tags in the details view does not contain the tag "tag4" # FilesAppContext::iSeeThatTheInputFieldForTagsInTheDetailsViewDoesNotContainTheTag()
    And I see that the input field for tags in the details view contains the tag "tag3"         # FilesAppContext::iSeeThatTheInputFieldForTagsInTheDetailsViewContainsTheTag()

@MorrisJobke
Copy link
Member

Failure is unrelated.

@MorrisJobke MorrisJobke merged commit 79ec7bb into nextcloud:master Mar 4, 2019
@MorrisJobke
Copy link
Member

/backport to stable15

@backportbot-nextcloud
Copy link

backport to stable15 in #14517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews feature: dav
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants