From 10a1c69fdff0a9af9925cc597d47d5bd00b19e7f Mon Sep 17 00:00:00 2001 From: duncanp-sonar Date: Wed, 8 Aug 2018 16:17:03 +0100 Subject: [PATCH] Fixed #96: reserved template manifest properties are preserved (#97) * Added jar manifest reader * Fixed #96: Reserved manifest properties are preserved --- PluginGenerator/JarManifestReader.cs | 83 ++++++++++++++++ .../SonarQube.Plugins.PluginGenerator.csproj | 1 + PluginGenerator/UIResources.Designer.cs | 20 +++- PluginGenerator/UIResources.resx | 6 ++ RoslynPluginGenerator/ArchiveUpdater.cs | 4 +- .../RoslynPluginJarBuilder.cs | 96 +++++++++++++------ RoslynPluginGenerator/UIResources.Designer.cs | 18 ++++ RoslynPluginGenerator/UIResources.resx | 6 ++ .../IntegrationTests/Roslyn/RoslynGenTests.cs | 49 +++++----- .../JarManifestReaderTests.cs | 88 +++++++++++++++++ ...arQube.Plugins.PluginGeneratorTests.csproj | 1 + 11 files changed, 318 insertions(+), 54 deletions(-) create mode 100644 PluginGenerator/JarManifestReader.cs create mode 100644 Tests/PluginGeneratorTests/JarManifestReaderTests.cs diff --git a/PluginGenerator/JarManifestReader.cs b/PluginGenerator/JarManifestReader.cs new file mode 100644 index 0000000..c5fd04b --- /dev/null +++ b/PluginGenerator/JarManifestReader.cs @@ -0,0 +1,83 @@ +/* + * SonarQube Roslyn SDK + * Copyright (C) 2015-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using System; +using System.Collections.Generic; + +namespace SonarQube.Plugins +{ + /// + /// Reads a valid v1.0 Jar Manifest. + /// See http://docs.oracle.com/javase/6/docs/technotes/guides/jar/jar.html#JAR%20Manifest + /// + public class JarManifestReader + { + private readonly Dictionary kvps; + private const string SEPARATOR = ": "; + + public JarManifestReader(string manifestText) + { + if (manifestText == null) + { + throw new ArgumentNullException(nameof(manifestText)); + } + + kvps = new Dictionary(StringComparer.OrdinalIgnoreCase); + + // A manifest file line can have at most 72 characters. Long values are split across + // multiple lines, with the continuation line starting with a single space. + // The simplest way to rejoin the lines is just to replace all (EOL + space) with EOL + var joinedText = manifestText.Replace("\r\n ", string.Empty); + var lines = joinedText.Split(new string[] { "\r\n" }, StringSplitOptions.RemoveEmptyEntries); + + // Every line should now be a key-value pair + foreach (var line in lines) + { + var index = line.IndexOf(SEPARATOR); + + if (index < 0) + { + throw new InvalidOperationException( + string.Format(System.Globalization.CultureInfo.CurrentCulture, UIResources.Reader_Error_InvalidManifest, line)); + } + + var key = line.Substring(0, index); + var value = line.Substring(index + SEPARATOR.Length); + kvps[key] = value; + } + } + + public string FindValue(string key) + { + kvps.TryGetValue(key, out string value); + return value; + } + + public string GetValue(string key) + { + if (!kvps.TryGetValue(key, out string value)) + { + throw new InvalidOperationException( + string.Format(System.Globalization.CultureInfo.CurrentCulture, UIResources.Reader_Error_MissingManifestSetting, key)); + } + return value; + } + } +} diff --git a/PluginGenerator/SonarQube.Plugins.PluginGenerator.csproj b/PluginGenerator/SonarQube.Plugins.PluginGenerator.csproj index 4dfcddb..8ccab95 100644 --- a/PluginGenerator/SonarQube.Plugins.PluginGenerator.csproj +++ b/PluginGenerator/SonarQube.Plugins.PluginGenerator.csproj @@ -57,6 +57,7 @@ + diff --git a/PluginGenerator/UIResources.Designer.cs b/PluginGenerator/UIResources.Designer.cs index 9b745bc..225baf0 100644 --- a/PluginGenerator/UIResources.Designer.cs +++ b/PluginGenerator/UIResources.Designer.cs @@ -19,7 +19,7 @@ namespace SonarQube.Plugins { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "15.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class UIResources { @@ -95,5 +95,23 @@ internal static string Misc_Error_InvalidRepositoryKey { return ResourceManager.GetString("Misc_Error_InvalidRepositoryKey", resourceCulture); } } + + /// + /// Looks up a localized string similar to Manifest file is not valid - line does not contain a key-value separator: '{0}'. + /// + internal static string Reader_Error_InvalidManifest { + get { + return ResourceManager.GetString("Reader_Error_InvalidManifest", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The expected setting was not found in the manifest file: {0}. + /// + internal static string Reader_Error_MissingManifestSetting { + get { + return ResourceManager.GetString("Reader_Error_MissingManifestSetting", resourceCulture); + } + } } } diff --git a/PluginGenerator/UIResources.resx b/PluginGenerator/UIResources.resx index cd60612..0049926 100644 --- a/PluginGenerator/UIResources.resx +++ b/PluginGenerator/UIResources.resx @@ -129,4 +129,10 @@ The attribute name is too long. Maximum 70 chars. Name: {0} + + Manifest file is not valid - line does not contain a key-value separator: '{0}' + + + The expected setting was not found in the manifest file: {0} + \ No newline at end of file diff --git a/RoslynPluginGenerator/ArchiveUpdater.cs b/RoslynPluginGenerator/ArchiveUpdater.cs index b696cc3..de46884 100644 --- a/RoslynPluginGenerator/ArchiveUpdater.cs +++ b/RoslynPluginGenerator/ArchiveUpdater.cs @@ -86,6 +86,8 @@ public void UpdateArchive() logger.LogDebug(UIResources.ZIP_JarUpdated, outputArchiveFilePath); } + #endregion Public methods + private void DoUpdate() { using (ZipArchive newArchive = new ZipArchive(new FileStream(outputArchiveFilePath, FileMode.Open), ZipArchiveMode.Update)) @@ -121,7 +123,5 @@ private ZipArchiveEntry GetOrCreateEntry(ZipArchive archive, string fullEntryNam return archive.CreateEntry(fullEntryName); } - - #endregion Public methods } } \ No newline at end of file diff --git a/RoslynPluginGenerator/RoslynPluginJarBuilder.cs b/RoslynPluginGenerator/RoslynPluginJarBuilder.cs index 384fa16..a6bb9da 100644 --- a/RoslynPluginGenerator/RoslynPluginJarBuilder.cs +++ b/RoslynPluginGenerator/RoslynPluginJarBuilder.cs @@ -21,6 +21,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.IO.Compression; using SonarQube.Plugins.Common; namespace SonarQube.Plugins.Roslyn @@ -35,16 +36,11 @@ public class RoslynPluginJarBuilder /// private const string TemplateJarResourceName = "SonarQube.Plugins.Roslyn.Resources.sonar-roslyn-sdk-template-plugin-1.1.jar"; - /// - /// The name of the plugin class in the embedded jar file - /// - private const string PluginClassName = "org.sonar.plugins.roslynsdk.RoslynSdkGeneratedPlugin"; - - // Locations in the jar where various file should be embedded - private const string RelativeManifestResourcePath = "META-INF\\MANIFEST.MF"; - - private const string RelativeConfigurationResourcePath = "org\\sonar\\plugins\\roslynsdk\\configuration.xml"; - private const string RelativeRulesXmlResourcePath = "org\\sonar\\plugins\\roslynsdk\\rules.xml"; + // Locations in the jar archive where various file should be embedded. + // Using forward-slash since that is the separator used by Java for archive entry names. + private const string RelativeManifestResourcePath = "META-INF/MANIFEST.MF"; + private const string RelativeConfigurationResourcePath = "org/sonar/plugins/roslynsdk/configuration.xml"; + private const string RelativeRulesXmlResourcePath = "org/sonar/plugins/roslynsdk/rules.xml"; private readonly ILogger logger; @@ -65,8 +61,6 @@ public RoslynPluginJarBuilder(ILogger logger) pluginProperties = new Dictionary(); jarManifestBuilder = new JarManifestBuilder(); fileToRelativePathMap = new Dictionary(); - - SetFixedManifestProperties(); } public RoslynPluginJarBuilder SetJarFilePath(string filePath) @@ -178,7 +172,6 @@ public RoslynPluginJarBuilder SetRulesFilePath(string filePath) return this; } - public RoslynPluginJarBuilder SetRepositoryKey(string key) { RepositoryKeyUtilities.ThrowIfInvalid(key); @@ -230,12 +223,13 @@ public void BuildJar(string workingDirectory) ValidateConfiguration(); + string templateJarFilePath = ExtractTemplateJarFile(workingDirectory); + // Create the config and manifest files string configFilePath = BuildConfigFile(workingDirectory); - string manifestFilePath = jarManifestBuilder.WriteManifest(workingDirectory); + string manifestFilePath = BuildManifest(templateJarFilePath, workingDirectory); // Update the jar - string templateJarFilePath = ExtractTemplateJarFile(workingDirectory); ArchiveUpdater updater = new ArchiveUpdater(this.logger); updater.SetInputArchive(templateJarFilePath) @@ -315,20 +309,6 @@ private string FindPluginKey() return pluginKey; } - /// - /// Sets the invariant, required manifest properties - /// - private void SetFixedManifestProperties() - { - // This property must appear first in the manifest. - // See http://docs.oracle.com/javase/6/docs/technotes/guides/jar/jar.html#JAR%20Manifest - jarManifestBuilder.SetProperty("Sonar-Version", "4.5.2"); - jarManifestBuilder.SetProperty("Plugin-Dependencies", "META-INF/lib/sslr-squid-bridge-2.6.jar"); - jarManifestBuilder.SetProperty("Plugin-SourcesUrl", "https://github.com/SonarSource-VisualStudio/sonarqube-roslyn-sdk-template-plugin"); - - jarManifestBuilder.SetProperty("Plugin-Class", PluginClassName); - } - private static string ExtractTemplateJarFile(string workingDirectory) { string templateJarFilePath = Path.Combine(workingDirectory, "template.jar"); @@ -345,6 +325,64 @@ private static string ExtractTemplateJarFile(string workingDirectory) return templateJarFilePath; } + private string BuildManifest(string templateJarFilePath, string workingDirectory) + { + var templateManifest = GetContentsOfFileFromArchive(templateJarFilePath, RelativeManifestResourcePath); + + CopyReservedPropertiesFromExistingManifest(templateManifest); + + var manifestFilePath = jarManifestBuilder.WriteManifest(workingDirectory); + return manifestFilePath; + } + + private string GetContentsOfFileFromArchive(string pathToArchive, string fullEntryName) + { + string text = null; + using (var archive = new ZipArchive(new FileStream(pathToArchive, FileMode.Open))) + { + var entry = archive.GetEntry(fullEntryName); + if (entry == null) + { + throw new InvalidOperationException( + string.Format(System.Globalization.CultureInfo.CurrentCulture, + UIResources.Builder_Error_EntryNotFoundInTemplatePlugin, fullEntryName)); + } + + var buffer = new byte[entry.Length]; + using (var entryStream = entry.Open()) + { + entryStream.Read(buffer, 0, buffer.Length); + text = System.Text.Encoding.UTF8.GetString(buffer); + } + } + + return text; + } + + /// + /// Some of the properties in the template manifest should + /// be preserved e.g. the supported SonarQube version + /// + private void CopyReservedPropertiesFromExistingManifest(string templateManifest) + { + var reader = new JarManifestReader(templateManifest); + CopyValueFromExistingManifest(reader, "Sonar-Version"); + CopyValueFromExistingManifest(reader, "Plugin-Dependencies"); + CopyValueFromExistingManifest(reader, "Plugin-Class"); + CopyValueFromExistingManifest(reader, "SonarLint-Supported"); // applies to other IDEs i.e. not VS + } + + private void CopyValueFromExistingManifest(JarManifestReader reader, string property) + { + if (jarManifestBuilder.TryGetValue(property, out string value)) + { + throw new InvalidOperationException( + string.Format(System.Globalization.CultureInfo.CurrentCulture, + UIResources.Builder_Error_ManifestPropertyShouldBeCopiedFromTemplate, property)); + } + jarManifestBuilder.SetProperty(property, reader.GetValue(property)); + } + #endregion Private methods configuration } } \ No newline at end of file diff --git a/RoslynPluginGenerator/UIResources.Designer.cs b/RoslynPluginGenerator/UIResources.Designer.cs index 1d1e099..f74b773 100644 --- a/RoslynPluginGenerator/UIResources.Designer.cs +++ b/RoslynPluginGenerator/UIResources.Designer.cs @@ -236,6 +236,24 @@ public static string APG_UsingSuppliedRulesFile { } } + /// + /// Looks up a localized string similar to Entry not found in template plugin: {0}. + /// + public static string Builder_Error_EntryNotFoundInTemplatePlugin { + get { + return ResourceManager.GetString("Builder_Error_EntryNotFoundInTemplatePlugin", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Manifest property should be copied from the template jar, not set directly: {0}. + /// + public static string Builder_Error_ManifestPropertyShouldBeCopiedFromTemplate { + get { + return ResourceManager.GetString("Builder_Error_ManifestPropertyShouldBeCopiedFromTemplate", resourceCulture); + } + } + /// /// Looks up a localized string similar to The file path for the jar file to be created must be specified. /// diff --git a/RoslynPluginGenerator/UIResources.resx b/RoslynPluginGenerator/UIResources.resx index 253b72b..ff786f2 100644 --- a/RoslynPluginGenerator/UIResources.resx +++ b/RoslynPluginGenerator/UIResources.resx @@ -179,6 +179,12 @@ Rules definitions: a template rules xml file for the analyzer was saved to {0}. Using the supplied rules xml file: {0} + + Entry not found in template plugin: {0} + + + Manifest property should be copied from the template jar, not set directly: {0} + The file path for the jar file to be created must be specified diff --git a/Tests/IntegrationTests/Roslyn/RoslynGenTests.cs b/Tests/IntegrationTests/Roslyn/RoslynGenTests.cs index d98c118..b2f33c4 100644 --- a/Tests/IntegrationTests/Roslyn/RoslynGenTests.cs +++ b/Tests/IntegrationTests/Roslyn/RoslynGenTests.cs @@ -247,6 +247,13 @@ private void CheckCanGenerateForThirdPartyAssembly(string packageId, SemanticVer // Expecting one plugin per dependency with analyzers AssertJarsGenerated(outputDir, 1); + + var jarFilePath = Directory.GetFiles(outputDir, "*.jar", SearchOption.TopDirectoryOnly).Single(); + var jarChecker = new ZipFileChecker(TestContext, jarFilePath); + var actualManifestFilePath = jarChecker.AssertFileExists("META-INF\\MANIFEST.MF"); + + JarManifestReader reader = new JarManifestReader(File.ReadAllText(actualManifestFilePath)); + AssertFixedValuesInManifest(reader); } /// @@ -320,10 +327,12 @@ private void CheckJarGeneratedForPackage(string rootDir, DiagnosticAnalyzer anal // Check the contents of the manifest string actualManifestFilePath = jarChecker.AssertFileExists("META-INF\\MANIFEST.MF"); - string[] actualManifest = File.ReadAllLines(actualManifestFilePath); - AssertExpectedManifestValue(WellKnownPluginProperties.Key, pluginId.Replace(".", ""), actualManifest); - AssertExpectedManifestValue("Plugin-Key", pluginId.Replace(".", ""), actualManifest); // plugin-key should be lowercase and alphanumeric - AssertPackagePropertiesInManifest(package, actualManifest); + + var manifestReader = new JarManifestReader(File.ReadAllText(actualManifestFilePath)); + manifestReader.FindValue(WellKnownPluginProperties.Key).Should().Be(pluginId.Replace(".", "")); + + AssertPackagePropertiesInManifest(package, manifestReader); + AssertFixedValuesInManifest(manifestReader); // Check the rules string actualRuleFilePath = jarChecker.AssertFileExists("." + config.RulesXmlResourcePath); @@ -373,27 +382,23 @@ private static void AssertRuleExists(DiagnosticDescriptor descriptor, Rules rule actual.Severity.Should().NotBeNull("Severity should be specified"); } - private static void AssertPackagePropertiesInManifest(IPackage package, string[] actualManifest) + private static void AssertPackagePropertiesInManifest(IPackage package, JarManifestReader manifestReader) { - AssertExpectedManifestValue("Plugin-Name", package.Title, actualManifest); - AssertExpectedManifestValue("Plugin-Version", package.Version.ToString(), actualManifest); - AssertExpectedManifestValue("Plugin-Description", package.Description, actualManifest); - AssertExpectedManifestValue("Plugin-Organization", String.Join(",", package.Owners), actualManifest); - AssertExpectedManifestValue("Plugin-Homepage", package.ProjectUrl.ToString(), actualManifest); - AssertExpectedManifestValue("Plugin-Developers", String.Join(",", package.Authors), actualManifest); - AssertExpectedManifestValue("Plugin-TermsConditionsUrl", package.LicenseUrl.ToString(), actualManifest); + manifestReader.FindValue("Plugin-Name").Should().Be(package.Title); + manifestReader.FindValue("Plugin-Version").Should().Be(package.Version.ToString()); + manifestReader.FindValue("Plugin-Description").Should().Be(package.Description); + manifestReader.FindValue("Plugin-Organization").Should().Be(String.Join(",", package.Owners)); + manifestReader.FindValue("Plugin-Homepage").Should().Be(package.ProjectUrl.ToString()); + manifestReader.FindValue("Plugin-Developers").Should().Be(String.Join(",", package.Authors)); + manifestReader.FindValue("Plugin-TermsConditionsUrl").Should().Be(package.LicenseUrl.ToString()); } - - private static void AssertExpectedManifestValue(string propertyName, string expectedValue, string[] actualManifest) + + private static void AssertFixedValuesInManifest(JarManifestReader reader) { - string expectedPrefix = propertyName + ": "; - - string match = actualManifest.SingleOrDefault(a => a.StartsWith(expectedPrefix, StringComparison.Ordinal)); - match.Should().NotBeNull("Failed to find expected manifest property: {0}", propertyName); - - // TODO: handle multi-line values - string actualValue = match.Substring(expectedPrefix.Length); - actualValue.Should().Be(expectedValue, "Unexpected manifest property value. Property: {0}", propertyName); + reader.FindValue("Sonar-Version").Should().Be("6.7"); + reader.FindValue("Plugin-Class").Should().Be("org.sonar.plugins.roslynsdk.RoslynSdkGeneratedPlugin"); + reader.FindValue("SonarLint-Supported").Should().Be("false"); + reader.FindValue("Plugin-Dependencies").Should().Be("META-INF/lib/jsr305-1.3.9.jar META-INF/lib/commons-io-2.6.jar META-INF/lib/stax2-api-3.1.4.jar META-INF/lib/staxmate-2.0.1.jar META-INF/lib/stax-api-1.0.1.jar"); } private void CheckEmbeddedAnalyzerPayload(ZipFileChecker jarChecker, string staticResourceName, diff --git a/Tests/PluginGeneratorTests/JarManifestReaderTests.cs b/Tests/PluginGeneratorTests/JarManifestReaderTests.cs new file mode 100644 index 0000000..6c0b9c7 --- /dev/null +++ b/Tests/PluginGeneratorTests/JarManifestReaderTests.cs @@ -0,0 +1,88 @@ +/* + * SonarQube Roslyn SDK + * Copyright (C) 2015-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; + +namespace SonarQube.Plugins.PluginGeneratorTests +{ + [TestClass] + public class JarManifestReaderTests + { + [TestMethod] + public void ReadSingleAndMultiLines() + { + // Arrange + var text = @"Manifest-Version: 1.0 + +Plugin-Dependencies: META-INF/lib/jsr305-1.3.9.jar META-INF/lib/common + s-io-2.6.jar META-INF/lib/stax2-api-3.1.4.jar META-INF/lib/staxmate-2 + .0.1.jar META-INF/lib/stax-api-1.0.1.jar +Plugin-SourcesUrl: https://github.com/SonarSource-VisualStudio/sonarqu + be-roslyn-sdk-template-plugin + + +"; + // Act + var jarReader = new JarManifestReader(text); + + // Assert + jarReader.FindValue("Manifest-Version").Should().Be("1.0"); + + // Multi-line value should be concatenated correctly + jarReader.FindValue("Plugin-Dependencies").Should().Be("META-INF/lib/jsr305-1.3.9.jar META-INF/lib/commons-io-2.6.jar META-INF/lib/stax2-api-3.1.4.jar META-INF/lib/staxmate-2.0.1.jar META-INF/lib/stax-api-1.0.1.jar"); + + // Multi-line with blank lines after - blanks ignored + jarReader.FindValue("Plugin-SourcesUrl").Should().Be(@"https://github.com/SonarSource-VisualStudio/sonarqube-roslyn-sdk-template-plugin"); + + // Not case-sensitive + jarReader.FindValue("MANIFEST-VERSION").Should().Be("1.0"); + } + + [TestMethod] + public void MissingSeparator_Throws() + { + // Arrange + var text = @"Manifest-Version: 1.0 +Line without a separator"; + + // Act + Action act = () => new JarManifestReader(text); + + // Assert + act.Should().ThrowExactly().And.Message.Should().Be("Manifest file is not valid - line does not contain a key-value separator: 'Line without a separator'"); + } + + [TestMethod] + public void Get_MissingSetting_Throws() + { + // Arrange + var text = @"Manifest-Version: 1.0"; + var jarReader = new JarManifestReader(text); + + // Act + Action act = () => jarReader.GetValue("missing-key"); + + // Assert + act.Should().ThrowExactly().And.Message.Should().Be("The expected setting was not found in the manifest file: missing-key"); + } + } +} diff --git a/Tests/PluginGeneratorTests/SonarQube.Plugins.PluginGeneratorTests.csproj b/Tests/PluginGeneratorTests/SonarQube.Plugins.PluginGeneratorTests.csproj index f155bfb..8f33f73 100644 --- a/Tests/PluginGeneratorTests/SonarQube.Plugins.PluginGeneratorTests.csproj +++ b/Tests/PluginGeneratorTests/SonarQube.Plugins.PluginGeneratorTests.csproj @@ -73,6 +73,7 @@ +